fix(ui): read the preset catalog at runtime in the dashboard tests - #39478
Conversation
The autoRouterPresets mock imported litellm/proxy/public_endpoints/autorouter_presets.json as a module. That path sits outside ui/litellm-dashboard, the only directory the UI Dockerfile copies, so `next build` type-checking inside the image failed with "Cannot find module" and the ui-image job went red on every PR that touched an image-scan path. Read the file with fs at runtime instead; vitest still derives expectations from the real bundled catalog. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HqEPCNLDrssxsuezhAaL4j
Greptile SummaryThe PR replaces a cross-package JSON module import with a test-runtime filesystem read anchored to the mock file, avoiding UI Docker type-check resolution failures while preserving catalog-derived expectations.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains; the runtime path is independent of the caller's working directory, the mock is not evaluated by the production build, and the existing catalog assertions remain intact.
|
| Filename | Overview |
|---|---|
| ui/litellm-dashboard/tests/mocks/autoRouterPresets.ts | Loads the real preset catalog at test runtime through a cwd-independent path; both previously reported concerns are resolved. |
| ui/litellm-dashboard/src/lib/autorouter_presets.test.ts | Reuses the mock's raw parsed catalog without changing assertions or weakening independent hydration coverage. |
Reviews (3): Last reviewed commit: "fix(ui): read the preset catalog through..." | Re-trigger Greptile
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HqEPCNLDrssxsuezhAaL4j
|
bugbot run |
|
@greptileai review |
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 fcc9b81. Configure here.
mateo-berri
left a comment
There was a problem hiding this comment.
LGTM. Thanks!
One tiny non-blocking nit
| // Derived from the real bundled catalog so a preset edit there flows into test expectations | ||
| // instead of redding on a stale copy. Exported as vi.fn so a test can override the query state. | ||
| export const BUNDLED_PRESETS = hydratePresets(bundledPresets as AutoRouterPresetsResponse); | ||
| const CATALOG_PATH = resolve(__dirname, "../../../../litellm/proxy/public_endpoints/autorouter_presets.json"); |
There was a problem hiding this comment.
nit: hmm I never really liked this drilling. Maybe it could be absolute...?
There was a problem hiding this comment.
Agreed it's not pretty, but I'd keep it: anchor-on-own-file-and-walk-up is the existing pattern on both sides of the repo, and there is no "absolute" alternative here that isn't machine-specific.
- TS:
scripts/gen-api-types.mjscomputesdashboardDirandrepoRootby walking up fromimport.meta.urlwith..hops;vitest.config.tsanchors its alias withresolve(__dirname, "src"). - Python: 37 tests use
Path(__file__).resolve().parents[2], 18 more usePath(__file__).parents[2] / "model_prices_and_context_window.json", and a handful goparents[3..5]. No shared repo-root helper exists. process.cwd()was the previous version of this line; Greptile flagged it (vitest from the repo root fails at import time), which is why fcc9b81 moved to__dirname.
The only dot-free option is a repo-root constant defined in vitest.config.ts plus a global .d.ts so next build still type-checks. Happy to add that once a second test needs a repo-root path, but for one consumer it's more machinery than the line it replaces.
|
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 f62e87d. Configure here.
TLDR
Problem this solves:
How it solves it:
ui/litellm-dashboard/any moreUser Flow
Before: an operator building the UI container from source never gets an image, because the build stops on a type error
litellm_internal_stagingand rundocker build -f ui/Dockerfile -t litellm-ui .from the repo rootnpm run buildprints✓ Compiled successfullyfollowed byRunning TypeScript ...Failed to type check.andType error: Cannot find module '../../../../litellm/proxy/public_endpoints/autorouter_presets.json' or its corresponding type declarations.docker buildexits 1 withprocess "/bin/sh -c npm run build" did not complete successfully, and nolitellm-uiimage is taggedhttp://localhost:3000/ui/refuses the connectionAfter: the same build finishes and the container serves the Admin UI
docker build -f ui/Dockerfile -t litellm-ui .from the repo rootnpm run buildprints✓ Compiled successfullyfollowed byRunning TypeScript ...docker buildexits 0 and tagslitellm-uidocker run -p 3000:3000 litellm-uiandhttp://localhost:3000/ui/serves the Admin UIRelevant issues
Linear ticket
Resolves LIT-6783
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)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
The failing job is
ui-imagein the Image Scan workflow, which runs the samedocker build -f ui/Dockerfile .an operator would. That workflow is path-filtered, so it does not run on this PR automatically; both runs below were started withgh workflow run image-scan.yml --ref <ref>on the exact commits named.Before (ff17e8b,
litellm_internal_staging)gh workflow run image-scan.yml --repo BerriAI/litellm --ref litellm_internal_stagingstarted run 33715609093ui-image,runtime-imageandimage-scan.migrations-image,gateway-imageandbackend-imagepassedui-imagelog shows the build dying in the type check:After (f62e87d)
gh workflow run image-scan.yml --repo BerriAI/litellm --ref litellm_ui_presets_mock_build_ctxstarted run 33716064367ui-imagelog now walks straight past the type check into the static export:add_auto_router_tab.test.tsxasserts the exact preset labels the shipped catalog carries (["Anthropic Family", "Gemini Family", "Lite", "OpenAI Family", "Custom Configuration"]), so a green run is also evidence the tests still read the real catalog rather than a stale copy.ui/litellm-dashboard/still resolves outside the dashboard rootGET /public/autorouter_presets, which this diff does not touchType
🐛 Bug Fix
Caveats (if any)
Low
ui/litellm-dashboard/autorouter_presets.test.tsis a plain unit test but now imports the vitest mock module for its datavi.fn()at import time, so the unit test pulls in a test double it never usesnext builddrops type errors coming from*.test.tsand*.spec.tsfilestests/mocks/is the file that surfaced it41 6 * * *run catches a break within a day.github/**, which needs two approvals, so it belongs in its own PRFinal Attestation