Skip to content

fix(litellm): pin proxy deps (litellm 1.95.0 + fastapi 0.140.0) to fix startup crash - #76

Merged
CarlesUIPath merged 3 commits into
mainfrom
fix/litellm-fastapi-version-pin
Aug 4, 2026
Merged

fix(litellm): pin proxy deps (litellm 1.95.0 + fastapi 0.140.0) to fix startup crash#76
CarlesUIPath merged 3 commits into
mainfrom
fix/litellm-fastapi-version-pin

Conversation

@CarlesUIPath

Copy link
Copy Markdown
Contributor

Problem

Starting the LiteLLM open-weight proxy via litellm/start-litellm.sh crashed with a masked error:

ModuleNotFoundError: No module named 'proxy_server'

Root cause: the launch used unpinned uvx --from 'litellm[proxy]'. litellm==1.95.0 declares fastapi>=0.136.3,<1.0, so uvx resolved the newest fastapi — but a 0.140.x patch removed fastapi.dependencies.utils.get_flat_dependant, which litellm's proxy still imports. The real ImportError is caught and re-raised as the misleading ModuleNotFoundError: proxy_server in proxy_cli.py::run_server. A <-range cap isn't enough because the break landed inside the allowed range.

Fix

  • Pin the proxy launch to litellm[proxy]==1.95.0 + fastapi==0.140.0 via uvx --from … --with … (an exact pin, since the break is intra-range).
  • Expose both as overridable env vars (LITELLM_SPEC / LITELLM_FASTAPI) so a future upgrade is a one-line override, not a code edit — mirroring the script's existing "${VAR:-default}" idiom.
  • Update the config's "Run manually:" comment to match the pinned command.
  • Surface the override vars in the header Usage block and echo the resolved pins at startup (proxy deps : …) for debuggability.

Scope

Ops-only shell + YAML-comment change. No Python, evaluation flow, models, criteria, or agents touched. Deliberately excludes the GPT/Codex + Gemini model routes (those live on feat/codex-through-claude-sdk).

Verification

  • Reproduced the crash on the unpinned spec (ImportError: get_flat_dependant → masked ModuleNotFoundError: proxy_server).
  • Pinned spec imports cleanly (LiteLLM: Current Version = 1.95.0, exit 0).
  • start-litellm.sh boots and serves: /health/liveliness"I'm alive!", all models registered.
  • Multi-reviewer code review: zero Critical/High/Medium findings; shell-safety confirmed (both vars double-quoted at the exec).

Note: make verify fails locally only on a pre-existing pyright error (Import "openai_codex" could not be resolved) present on main and untouched here; ruff check — the gate relevant to this diff — passes clean.

🤖 Generated with Claude Code

CarlesUIPath and others added 2 commits August 4, 2026 11:19
… startup

Unpinned `uvx --from 'litellm[proxy]'` resolves the newest fastapi, but a
0.140.x patch removed `get_flat_dependant`, which litellm 1.95.0's proxy
imports — so startup died with a masked `ModuleNotFoundError: proxy_server`.
Pin both deps (overridable via LITELLM_SPEC / LITELLM_FASTAPI) and update the
config's manual-run comment to match.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Address code-review notes: list LITELLM_SPEC / LITELLM_FASTAPI in the header
Usage block and echo the resolved pins in the startup banner so an operator
debugging a startup failure can see which versions actually ran.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown

Claude finished @CarlesUIPath's task in 1m 14s —— View job


Code Review in Progress

Review Tasks:

  • Read .github/code_review.md for review guidelines
  • Read CLAUDE.md for project conventions
  • Review the full diff (git diff origin/main...HEAD)
  • Read complete changed files for context
  • Perform cross-file consistency checks
  • Check for missing elements
  • Provide structured review feedback

@uipreliga uipreliga left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review: coder_eval — pr:76

Scope: pr:76 · branch fix/litellm-fastapi-version-pin (PR head; local branch = main) · c03dbcf · 2026-08-04T10:44Z · workflow variant

Change class: simple — pins two proxy dependency versions in an ops launcher script (plus matching comment updates); no Python, no evaluation flow, and the effect of the ${VAR:-default} pins at the exec line is self-evident at a glance

The harness core is in excellent shape — Type Safety, Security, Architecture, and Error Handling are clean at 10/10, and no finding in this review can change a task's score or final_status for identical agent output — with all remaining risk confined to the dev-time LiteLLM sidecar, where hand-duplicated dependency pins have no mechanical sync guard and three doc surfaces have already drifted apart; bottom line: ship it, then close the pin-drift gap with one cheap offline test.

Summary

Axis Score 🔴 🟠 🟡 🔵 Top Issue
1. Code Quality & Style 9.9 / 10 0 0 0 1 New proxy deps : status line copies the aligned label block's format but prints detached from it, after the banner heredoc
2. Type Safety 10 / 10 0 0 0 0
3. Test Health 9.5 / 10 0 0 1 0 Pin literals for the new pinned-launch contract are hand-duplicated across litellm/ files with no mechanical guard (existing tests/test_litellm_config.py is the precedent), and sibling copies are already stale
4. Security 10 / 10 0 0 0 0
5. Architecture & Design 10 / 10 0 0 0 0
6. Error Handling & Resilience 10 / 10 0 0 0 0
7. API Surface & Maintainability 9.9 / 10 0 0 0 1 LITELLM_FASTAPI holds a full PEP 508 requirement but its name reads as a version knob
8. Evaluation Harness Quality 9.9 / 10 0 0 0 1 New LITELLM_SPEC / LITELLM_FASTAPI knobs documented in the script header but not rippled into litellm/README.md's env-override list

Overall Score: 9.9 / 10 · Weakest Axis: Test Health at 9.5 / 10
Totals: 🔴 0 · 🟠 0 · 🟡 1 · 🔵 3 across 8 axes.

Blockers

None.

Non-blocking, but please consider before merge

  1. [Axis 3] Pin literals for the new pinned-launch contract are hand-duplicated across litellm/ files with no mechanical guard (existing tests/test_litellm_config.py is the precedent), and sibling copies are already stale (litellm/start-litellm.sh:107) — litellm/start-litellm.sh:107-110 now carries the ONLY machine-read record of the working sidecar pins:
LITELLM_SPEC="${LITELLM_SPEC:-litellm[proxy]==1.95.0}"
LITELLM_FASTAPI="${LITELLM_FASTAPI:-fastapi==0.140.0}"
echo "proxy deps : $LITELLM_SPEC + $LITELLM_FASTAPI"
exec uvx --from "$LITELLM_SPEC" --with "$LITELLM_FASTAPI" litellm --config "$CONFIG" --host 127.0.0.1 --port "$PORT"

Nothing mechanical covers it: grep -rn 'start-litellm\|litellm\[proxy\]\|LITELLM_SPEC\|LITELLM_FASTAPI' tests/ .github/ Makefile returns no matches (exit 1), .pre-commit-config.yaml has no shellcheck/shell hook, and no workflow or Dockerfile references litellm at all. The two Python-side tests that do touch this backend would not have caught the crash: tests/test_docker_litellm_env.py only asserts _rewrite_loopback_for_container("http://localhost:4000") == "http://host.docker.internal:4000" plus env forwarding; the cost tests never launch a proxy or resolve a dependency spec.

That absence matters because the repo already established the opposite convention for this exact directory: tests/test_litellm_config.py (added in 4131a2a) is a pure offline shape guard on litellm/litellm-config.yaml, with the docstring rationale "A typo in this YAML ... fails silently at proxy startup rather than in CI" — verbatim the failure mode this PR is fixing. The new pins got no equivalent.

The drift is not hypothetical — this PR updated only 1 of 3 committed copies of the launch invocation, leaving two stale at HEAD: litellm/cost_logger.py:29 still documents uvx --from 'litellm[proxy]' litellm unpinned, and litellm/README.md:57-58 still reads "Overridable via env: LITELLM_PORT (default 4000), LITELLM_CONFIG, ENV_FILE, LITELLM_MASTER_KEY." — omitting the new LITELLM_SPEC / LITELLM_FASTAPI that start-litellm.sh:15-16 now advertises.

Fix (cheap, offline, mirrors the existing module): add a case to tests/test_litellm_config.py that reads litellm/start-litellm.sh, regex-extracts the LITELLM_SPEC= / LITELLM_FASTAPI= defaults, and asserts both exact == pins also appear in the # Run manually comment of litellm/litellm-config.yaml (lines 21-24), so a future pin bump cannot silently desync the two in-scope files; add a subprocess.run(["bash", "-n", "litellm/start-litellm.sh"]) syntax assertion alongside it. An actual boot smoke (resolve the spec, import litellm.proxy.proxy_server) needs network and a full uvx resolve — that belongs in an optional scheduled CI job, not the unit suite, and is not warranted on its own for a dev-time sidecar.

Severity rationale (Medium, not High): start-litellm.sh is a dev-time sidecar launcher outside the harness's public Python API — absent from CI, the nightly path, and the Docker driver — so this matches the Axis 3 🟡 anchor "coverage gap on a non-critical module", not 🟠 "untested public API".

Nits

  1. [Axis 1] New proxy deps : status line copies the aligned label block's format but prints detached from it, after the banner heredoc (litellm/start-litellm.sh:109) — echo "proxy deps : $LITELLM_SPEC + $LITELLM_FASTAPI" uses the exact 11-column label padding of the preflight status block (echo "config : $CONFIG"echo "cost log : $LITELLM_COST_LOG", lines 73-77), but is emitted at line 109 — after the stale-listener kill and after the cat <<EOF banner that ends with "# then run: coder-eval run --model zai.glm-5". The alignment is therefore invisible on screen: the operator sees five aligned lines, a banner, then one orphaned aligned line. Move the two assignments and the echo up next to lines 73-77 (they have no dependency on anything between), leaving only exec uvx --from "$LITELLM_SPEC" --with "$LITELLM_FASTAPI" ... plus its explanatory comment at the bottom.
  2. [Axis 7] LITELLM_FASTAPI holds a full PEP 508 requirement but its name reads as a version knob (litellm/start-litellm.sh:108) — LITELLM_FASTAPI="${LITELLM_FASTAPI:-fastapi==0.140.0}" (line 108) holds an entire requirement specifier, exactly like LITELLM_SPEC="${LITELLM_SPEC:-litellm[proxy]==1.95.0}" (line 107) — but the two names read as different shapes (_SPEC says "specifier"; a bare _FASTAPI reads as "the fastapi version"), inviting LITELLM_FASTAPI=0.140.13, which becomes uvx --with "0.140.13" and fails with an unrelated "package not found" error rather than a pin error. The value shape is disclosed only in the comment at line 106 (# Override for an upgrade: LITELLM_SPEC='litellm[proxy]==<ver>' LITELLM_FASTAPI='fastapi==<ver>'.), 90 lines below the Usage header at line 16 that advertises the var. Rename to LITELLM_FASTAPI_SPEC for symmetry with LITELLM_SPEC, and/or inline the shape into the Usage header at line 16 (e.g. LITELLM_SPEC / LITELLM_FASTAPI (full pip specifiers, e.g. 'fastapi==0.140.0')). The failure mode is loud, not silent, which is why this is a nit rather than a correctness issue.
  3. [Axis 8] New LITELLM_SPEC / LITELLM_FASTAPI knobs documented in the script header but not rippled into litellm/README.md's env-override list (litellm/start-litellm.sh:15) — The script's own Usage header was updated (lines 15-16: # LITELLM_PORT (default 4000), LITELLM_CONFIG, ENV_FILE, LITELLM_MASTER_KEY, / # LITELLM_SPEC / LITELLM_FASTAPI (proxy dep pins — see the launch comment below)) and litellm-config.yaml's 'Run manually' comment was updated, but the operator-facing guide the README points every user at still reads Overridable via env: + 'LITELLM_PORT (default 4000), LITELLM_CONFIG, ENV_FILE, LITELLM_MASTER_KEY.' (litellm/README.md:57-58) — i.e. the documented upgrade path for the pins exists only inside the shell source. Add the two names to that README line so the third doc surface matches the two this PR updated.

What's Missing

Parallel paths:

  • 🟡 litellm/cost_logger.py:29 still documents the launch command as uvx --from 'litellm[proxy]' litellm — unpinned. The PR updated 2 of the 3 committed copies of that invocation (start-litellm.sh:110, litellm-config.yaml:21-24), leaving the cost-logger docstring showing the exact resolution that dies with the masked ModuleNotFoundError: proxy_server. Add the ==1.95.0 / --with 'fastapi==0.140.0' form there too. (trigger: litellm/start-litellm.sh) (restates: Axis 3: Pin literals hand-duplicated across litellm/ files with no mechanical guard)
  • 🔵 litellm/litellm-config.yaml:10-14 still lists three launch paths — "Manual", "Autostart: coder_eval spawns the proxy against this same file", and "an always-on docker sidecar mounts it" — but only the Manual copy (21-24) got the pin. Any non-start-litellm.sh launcher (a sidecar image, an autostart wrapper downstream) resolves litellm/fastapi unpinned and reproduces the identical startup break this PR is fixing. Either state in that block that the pin applies to every launch path, or drop the aspirational paths (no autostart/sidecar code exists in-repo — grep for start-litellm/litellm[proxy] finds none). (trigger: litellm/litellm-config.yaml)
  • 🔵 litellm/README.md:57-58 — the operator-facing "Overridable via env" list still stops at LITELLM_MASTER_KEY and omits the two knobs the script header now advertises, so the documented upgrade path for the pins exists only inside the shell source. (trigger: litellm/start-litellm.sh) (restates: Axis 8: New LITELLM_SPEC / LITELLM_FASTAPI knobs not rippled into litellm/README.md)
  • 🔵 litellm/README.md's Troubleshooting table (lines 188-193) gains no row for the failure this PR exists to fix. It already covers "proxy not reachable", "Invalid model name", and HTTP 401, but an operator hitting the masked ModuleNotFoundError: proxy_server (stale checkout, or a hand-run unpinned uvx, or a bad LITELLM_SPEC override) has no entry pointing at the fastapi pin — the diagnosis lives only in a shell comment at start-litellm.sh:100-106. Add one row: symptom ModuleNotFoundError: proxy_server / masked startup death → cause: fastapi drifted past 0.140.0 (get_flat_dependant removed); fix: use start-litellm.sh or pin fastapi==0.140.0. (trigger: litellm/start-litellm.sh)

Tests:

  • 🟡 No mechanical guard exists for the new pinned-launch contract: grep -rn 'start-litellm|litellm\[proxy\]|LITELLM_SPEC|LITELLM_FASTAPI' tests/ .github/ Makefile returns nothing, .pre-commit-config.yaml has no shellcheck/shfmt hook, and no workflow touches litellm/ at all — so neither the pin literals' duplication across start-litellm.sh:107-108 and litellm-config.yaml:23 nor a shell syntax error in the edited launch line is caught anywhere. The directory already has the right precedent (tests/test_litellm_config.py, added in 4131a2a, for exactly the "fails silently at proxy startup rather than in CI" reason); the new pins got no equivalent. Cheapest fix: a case in that module that regex-extracts the two == defaults from the script and asserts both appear in the config's "Run manually" comment, plus a subprocess.run(['bash','-n','litellm/start-litellm.sh']) assertion. (trigger: litellm/start-litellm.sh) (restates: Axis 3: Pin literals hand-duplicated across litellm/ files with no mechanical guard)
  • 🔵 The diff adds two new override branches (LITELLM_SPEC / LITELLM_FASTAPI, lines 107-108) with zero validation and zero coverage: whatever the operator exports is interpolated straight into uvx --from/--with, so a natural-reading LITELLM_FASTAPI=0.140.13 becomes uvx --with '0.140.13' and fails deep in uv resolution rather than as a pin error. A one-line guard in the script (reject a value not matching ^[A-Za-z0-9._-]+(\[[^]]+\])?[=<>!~]=) would make the new branch self-checking without needing a test harness for shell. (trigger: litellm/start-litellm.sh) (restates: Axis 7: LITELLM_FASTAPI holds a full PEP 508 requirement but its name reads as a version knob)
  • 🔵 Nothing will ever refresh these shell-embedded pins. .github/dependabot.yml covers only pip at / (pyproject), github-actions, and npm at /evalboard — and its pip block explicitly ignores minor/patch — while pip-audit/osv-scanner see only the harness's own dependency set, never the uvx-resolved sidecar env. So fastapi==0.140.0 and litellm[proxy]==1.95.0 will fossilize with no CVE visibility and no scheduled re-check; the comment at line 100-106 explains why the pin exists but records no criterion for when to lift it. Add a # re-verify by <date> / when litellm >= X drops the get_flat_dependant import note, or a uv-locked sidecar requirements file that dependabot can actually see. (trigger: litellm/start-litellm.sh)

Downstream consumers:

  • 🔵 The LITELLM_SPEC override is now the documented upgrade path for a proxy whose version is also the compatibility anchor for litellm/cost_logger.py's callback contract (callbacks: cost_logger.proxy_handler_instance + the OpenRouter usage.cost passthrough). If a bumped litellm changes that callback surface, the failure is silent, not loud: litellm_cost.load_cost_records returns [] for a missing/empty JSONL and apply_actual_cost no-ops, so the run falls back to static pricing.py numbers that look plausible in reports. Neither the launch comment (100-106) nor litellm/README.md says a LITELLM_SPEC bump must be re-validated against the cost join (e.g. "after bumping, confirm $LITELLM_COST_LOG grows and the run shows provider_call_costs"). (trigger: litellm/start-litellm.sh)

Harness & Lint Improvements

Static checks (lint / type):

  • [ce-lint] CE032 — a pinned dependency literal for the LiteLLM sidecar may exist in exactly one place. Pattern forbidden: any committed file under litellm/ restating a litellm[proxy]==<ver> / fastapi==<ver> pin (or a uvx --from ... litellm launch line) whose pins disagree with the LITELLM_SPEC= / LITELLM_FASTAPI= defaults in litellm/start-litellm.sh. Implementation: new module tests/lint/litellm_sidecar_parity.py — regex-extract the two ${VAR:-<default>} defaults from start-litellm.sh (the SSOT), then scan litellm/** (litellm-config.yaml, README.md, cost_logger.py) for lines matching uvx --from or a bare litellm[proxy]/fastapi requirement token and assert every occurrence carries the exact same == pin. Two extra assertions in the same pass, both cheap: (a) each extracted default must parse as a packaging.Requirement (packaging 26.0 is already available via pytest), so LITELLM_FASTAPI=0.140.13 — a bare version that uvx --with would reject as an unresolvable package — fails in CI rather than at the operator's terminal; (b) any such variable whose default parses as a Requirement must be named *_SPEC, enforcing the naming convention the A7 finding asks for instead of documenting it. Wire as tests/test_custom_lint.py::TestCE032LitellmSidecarPins (a @pytest.mark.lint class, NOT a BaseRule in tests/lint/runner.py — that runner is AST-only over .py, while this reasons over .sh/.yaml/.md, exactly like CE027–CE031). Next free id: CE001–CE025 and CE027–CE031 are in use, CE026 is retired. Boundary note: ruff/pyright/bandit cannot reach any of this — all four surfaces are shell, YAML comments, and Markdown, none of which those tools parse. Prevents: The merged A1/A3/A5/A7 cluster (litellm/start-litellm.sh:107 + litellm/litellm-config.yaml:23): pins duplicated across files with nothing enforcing sync. It fires today on the stale sibling the finding cites — litellm/cost_logger.py:29 (unpinned uvx --from 'litellm[proxy]' litellm) — which the finding's own narrower fix (a pytest limited to the two in-scope files) would miss. Assertions (a)/(b) prevent the A7 LITELLM_FASTAPI naming / value-shape finding (start-litellm.sh:108).
  • [ce-lint] CE033 — an env knob a committed operator script reads must be documented on every surface that lists that script's knobs. Pattern forbidden: a ${NAME:-default} read in a first-party .sh that is absent from either the script's own Usage: / Overridable via env: header or the sibling README.md's env-override list. Implementation: reuse the tests/lint/litellm_sidecar_parity.py parser — collect ${NAME:-...} names per script from an explicit registry (litellm/start-litellm.shlitellm/README.md; docker/coder_eval_entrypoint.sh and docker/coder_eval_runtime_entrypoint.sh → their documented surface), skipping internal locals via an EXEMPT map with a reason (same escape hatch as CE030/CE031). Assert set equality in both directions so an undocumented knob AND a documented-but-removed knob both fail. Wire as tests/test_custom_lint.py::TestCE033ScriptEnvDocParity. This is the inverse direction of CE027 (tests/lint/doc_env_parity.py, 'documented env var must have a consumer'); CE027 cannot cover it as-is because it scans only copy-pasteable NAME=value assignments under a registered framework prefix, whereas the README states these knobs as backticked prose under a non-registered LITELLM_ prefix — so CE027 stays silent by construction. Prevents: The merged A5/A7/A8 doc-drift finding (litellm/start-litellm.sh:15-16 advertises LITELLM_SPEC / LITELLM_FASTAPI, but litellm/README.md:57-58's parallel 'Overridable via env' list still stops at LITELLM_MASTER_KEY). Also generalizes the guard to the two Docker entrypoints, which sit on the nightly run path and carry the same undocumented-knob exposure today.

Harness improvements (not statically reachable):

  • Add a shell gate: a local pre-commit hook plus a make check step running bash -n and shellcheck over git ls-files '*.sh' (4 first-party scripts today: litellm/start-litellm.sh, scripts/check_commit_msg.sh, and the two docker/*_entrypoint.sh files that are load-bearing for the DockerRunner nightly path). .pre-commit-config.yaml has ruff/pyright/YAML/private-key hooks but zero shell coverage, and no workflow or Makefile target touches a .sh at all — first-party shell is an unlinted surface in a repo that ships operator scripts. Why not static: The CE runner is a Python-AST walker and ruff/pyright only parse .py; catching shell defects needs an external shell parser, so this can only be a tooling/CI gate, not a CEnnn or ruff rule. Precise about reach: shellcheck would NOT have caught A1 (an echo printed after the banner heredoc is an output-legibility nit, not a shell defect) — it catches the surrounding class in the same edited block (unquoted expansions, set -euo pipefail interactions, unused/masked vars) and delivers the bash -n syntax assertion the A3 finding requests, at repo scope rather than one file. Prevents: A1 (litellm/start-litellm.sh:109) only partially — the class around it, not the finding itself; plus the untested-shell gap A3 documents (grep -rn 'start-litellm' tests/ .github/ Makefile returns nothing at HEAD).
  • Add an optional scheduled (weekly + workflow_dispatch, non-blocking / continue-on-error) CI job that actually exercises the pinned sidecar: resolve the two specs and import the module that broke — uvx --from "$LITELLM_SPEC" --with "$LITELLM_FASTAPI" python -c 'import litellm.proxy.proxy_server' — then boot the proxy against litellm/litellm-config.yaml on an ephemeral port and probe /health with no credentials (a 401 proves the process is up). Keep it out of make test and the PR-blocking suite: this is a dev-time sidecar, and a network-dependent job must not gate merges. Why not static: Needs network and a full uvx dependency resolve. Any offline check — CE032 included — can only prove the literals agree with each other; it can never prove the pinned combination still imports. This is the one gate that would have caught the original masked ModuleNotFoundError: proxy_server, and the only one that will catch the next upstream drift (a yanked release, a pin that stops resolving, a transitive break). Prevents: Recurrence of the root defect this PR fixes by hand; complements A3, which explicitly scopes the boot smoke out of the unit suite.
  • Eliminate the duplication class instead of policing it: move the pins into one sourced file (litellm/pins.env holding LITELLM_SPEC= / LITELLM_FASTAPI=, sourced by start-litellm.sh preserving the ${VAR:-...} override semantics), and change the three doc copies (litellm-config.yaml's 'Run manually' comment, litellm/README.md, cost_logger.py's docstring) to point at that file rather than restate the literals. If adopted, CE032 shrinks to its Requirement-shape/naming assertions plus a 'no bare litellm[proxy]== / fastapi== literal outside pins.env' check — a strictly simpler rule than N-way literal comparison. Why not static: This is a change to how the source is laid out, not a check over it. A lint rule can only detect drift between copies that already exist; removing the copies removes the failure mode, and the residual rule then guards a single-writer invariant instead of an N-way sync. Prevents: The structural cause behind the merged A1/A3/A5/A7 pin cluster and the A5/A7/A8 README drift — three of the four confirmed findings are instances of 'the same literal or knob list is restated on 3-4 surfaces'.

Top 5 Priority Actions

  1. Add the missing mechanical guard for the new sidecar pins: extend /Users/religa/src/coder_eval/tests/test_litellm_config.py with a case that regex-extracts the LITELLM_SPEC / LITELLM_FASTAPI defaults from litellm/start-litellm.sh:107-108 and asserts both exact == pins also appear in the '# Run manually' comment at litellm/litellm-config.yaml:21-24, plus a bash -n litellm/start-litellm.sh syntax assertion — the only medium finding, and the sole thing standing between a future pin bump and a silent desync that fails at proxy startup rather than in CI.
  2. Ripple the two new knobs into the third, operator-facing doc surface: /Users/religa/src/coder_eval/litellm/README.md:57-58 still lists only LITELLM_PORT / LITELLM_CONFIG / ENV_FILE / LITELLM_MASTER_KEY while start-litellm.sh:15-16 now advertises LITELLM_SPEC and LITELLM_FASTAPI, so the documented upgrade path for the pins exists only inside the shell source (drift raised independently by three axes).
  3. Refresh the stale unpinned invocation in /Users/religa/src/coder_eval/litellm/cost_logger.py:29, which still documents uvx --from 'litellm[proxy]' litellm — the third committed copy of the launch command, and the one the proposed pin-sync test deliberately will not cover, so it needs a manual edit now rather than waiting on the guard.
  4. Rename LITELLM_FASTAPI to LITELLM_FASTAPI_SPEC at /Users/religa/src/coder_eval/litellm/start-litellm.sh:108 for symmetry with LITELLM_SPEC (line 107) and inline the value shape into the Usage header at line 16, so an operator does not set LITELLM_FASTAPI=0.140.13 and hit an unrelated 'package not found' from uvx --with 0.140.13 instead of a pin error.
  5. Move the proxy deps : echo and its two assignments from /Users/religa/src/coder_eval/litellm/start-litellm.sh:107-109 up beside the aligned preflight status block at lines 73-77 (they have no dependency on anything in between), so the 11-column label alignment is actually visible instead of one orphaned line printed after the banner heredoc.

Stats: 0 🔴 · 0 🟠 · 1 🟡 · 3 🔵 across 8 axes reviewed.

…pple

Apply code-review feedback on the proxy dep-pin fix:
- Rename LITELLM_FASTAPI -> LITELLM_FASTAPI_SPEC for symmetry with LITELLM_SPEC
  and add a fail-loud guard rejecting a bare version (the `=0.140.13` footgun).
- Move the pin defs + `proxy deps` echo up into the aligned preflight status
  block; leave only `exec` at the bottom.
- Ripple the new knobs into litellm/README.md (env-override list + a
  Troubleshooting row for the masked ModuleNotFoundError) and refresh the stale
  unpinned invocation in cost_logger.py's docstring.
- Add tests/test_litellm_config.py::TestLitellmSidecarPins: start-litellm.sh is
  the SSOT for the pins; assert both exact pins are restated in the config
  comment and cost_logger docstring, that defaults are full pip specifiers, and
  that the launcher passes `bash -n`.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@CarlesUIPath

CarlesUIPath commented Aug 4, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the thorough review. Addressed in b7b4a98:

Medium (Axis 3 — no mechanical guard for the pins): Added tests/test_litellm_config.py::TestLitellmSidecarPinsstart-litellm.sh is now the SSOT; the tests regex-extract the LITELLM_SPEC / LITELLM_FASTAPI_SPEC defaults and assert both exact == pins are restated verbatim in the config's # Run manually comment and the cost_logger.py docstring, that each default is a full pip specifier (name ends in _SPEC, value carries a comparator), and that the launcher passes bash -n. This closes the "pin bump silently desyncs the copies" gap across all three surfaces.

Top-5 #3 (stale unpinned copy): cost_logger.py:29 docstring updated to the pinned uvx --from 'litellm[proxy]==1.95.0' --with 'fastapi==0.140.0' litellm form (now covered by the sync test above).

Top-5 #2 / Axis 8 (README ripple): Added LITELLM_SPEC / LITELLM_FASTAPI_SPEC to the README "Overridable via env" list, and a Troubleshooting row for the masked ModuleNotFoundError: proxy_server → fastapi drift.

Top-5 #4 / Axis 7 (naming + footgun): Renamed LITELLM_FASTAPILITELLM_FASTAPI_SPEC for symmetry, inlined the specifier shape into the Usage header, and added a fail-loud guard that rejects a bare version (e.g. 0.140.13) before it reaches uvx --with.

Top-5 #5 / Axis 1 (orphaned echo): Moved the two pin assignments + the proxy deps : echo up beside the aligned preflight status block; only exec remains at the bottom.

Not taken (with rationale):

  • CE032/CE033 lint rules + pins.env extraction: the offline pytest guard already covers the drift across all three committed copies; a full whole-tree lint rule / single-sourced pins.env is a larger structural change I'd rather not fold into this bugfix PR.
  • Scheduled boot-smoke CI job: agreed it's the only thing that catches the next upstream drift, but it needs network + a full uvx resolve, so it belongs in an optional scheduled workflow rather than this PR.

Re-verified: 6/6 tests pass, bash -n clean, and the proxy still boots + serves (/health/liveliness"I'm alive!") with the proxy deps line now aligned in the status block.

@uipreliga
uipreliga self-requested a review August 4, 2026 15:04

@uipreliga uipreliga left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fix what you agree with and 🚢

@CarlesUIPath
CarlesUIPath merged commit f2f8580 into main Aug 4, 2026
13 checks passed
@CarlesUIPath
CarlesUIPath deleted the fix/litellm-fastapi-version-pin branch August 4, 2026 15:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants