refactor(ui): migrate memory page to shadcn - #34366
Conversation
…rating Adds role/text-based coverage for MemoryDetailDrawer (which had none) and extends MemoryView's test past the mocked table to the header, the create modal trigger and the detail drawer round trip. Both are green against the current antd components, so they act as an unedited regression net for the shadcn migration that follows.
Replaces the antd Drawer with ui/sheet and the antd Button, Typography and Space usage with ui/button plus token utilities, and swaps the @ant-design PlusOutlined icon for lucide's Plus. Toasts now go through the shared MessageManager so the route no longer imports antd directly. The route's tests were written against the antd components in the previous commit and are unchanged here, so they pass on both implementations. MemoryEditModal is left alone because it is built on antd Form; the table already sits on the shared DataTable.
…itellm_/migrate-page-memory-9b2c09 # Conflicts: # ui/litellm-dashboard/eslint-suppressions.json
Greptile SummaryMigrates the memory page and detail drawer from Ant Design to shared shadcn components
Confidence Score: 5/5The PR appears safe to merge No blocking failure remains in the responsive sheet fix
|
| Filename | Overview |
|---|---|
| ui/litellm-dashboard/src/app/(dashboard)/memory/_components/MemoryDetailDrawer.tsx | The responsive full-width sheet classes address the previously reported narrow-viewport overflow |
| ui/litellm-dashboard/src/app/(dashboard)/memory/_components/MemoryView.tsx | Replaces Ant Design layout, typography, button, icon, and notification usage without changing the page data flow |
| ui/litellm-dashboard/src/app/(dashboard)/memory/_components/MemoryDetailDrawer.test.tsx | Adds focused coverage for drawer rendering, optional fields, metadata, timestamps, and closure |
| ui/litellm-dashboard/src/app/(dashboard)/memory/_components/MemoryView.test.tsx | Adds coverage for page headings, create-modal activation, and detail-sheet interaction |
Reviews (2): Last reviewed commit: "fix(ui): keep the memory detail sheet in..." | Re-trigger Greptile
…creens The migrated sheet asked for a flat 720px width while its only max-width came from the primitive's sm:-scoped rule, so below the sm breakpoint no cap applied at all: on a 375px viewport the sheet rendered 720px wide with its left edge at -305px, and because it is position:fixed there was no scroll to reach the hidden content. The primitive's own w-3/4 default did not have this problem; the fixed pixel width is what removed the guard. Caps the width to the viewport at every breakpoint and only asks for 720px from sm up. Verified in a browser at 375px, 700px and 1280px: the sheet is now 375, 700 and 720 wide respectively, always at left 0.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
QA — Memory page shadcn migrationTested against a live proxy (port 4000, real Postgres 16) and the dashboard dev server (route Results
Detail drawer Narrow viewport (~560px), Sheet full-width and inside the viewport More evidence (page, overflow menu, create modal, recording)Memory page Overflow menu Create modal Recording Not covered: non-admin role gating (admin master-key login only) and create/edit/delete write paths beyond opening their modals, kept out to preserve the seeded state; both are out of scope for this markup-only migration Tested by Devin on behalf of @yuneng — session: https://app.devin.ai/sessions/580facfbbac041c2858d88fc633c1644 |
…itellm_/migrate-page-memory-9b2c09
…itellm_/migrate-page-memory-9b2c09
TLDR
Problem this solves:
How it solves it:
ui/sheet, button toui/button, typography to tokensRelevant issues
Linear ticket
Pre-Submission checklist
Please complete all items before asking a LiteLLM maintainer to review your PR
@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 tests are the regression evidence and they are deliberately split across two commits.
9dda5d882cadds them against the antd components, where they pass;d2f872d04fmigrates and does not touch the test files.git diff --name-only d2f872d04f~1 d2f872d04f -- '*test*'is empty, so the same unedited assertions are green on both implementationsScreenshots to attach, captured at commit
d2f872d04f:/ui/memoryTo reproduce by hand against a local proxy:
/v1/memoryscope note, with "New memory" on the rightType
🧹 Refactoring
Changes
Two files migrated, both owned exclusively by this route:
MemoryDetailDrawer.tsx: antdDrawertoui/sheet,SpaceandTypographyto flex and token utilities. Width stays 720px viadata-[side=right]overrides so tailwind-merge reconciles them against the primitive's ownsm:max-w-smMemoryView.tsx: antdButtontoui/button,TitleandParagraphto a plainh1andpwith token classes,Spaceto flex utilities, and@ant-design/iconsPlusOutlinedto lucidePlus. Toasts move from the raw antdmessageAPI to the sharedMessageManager, which is the wrapper 100+ other files already use; behaviour is identical and the route stops importing antd directlyDeliberately not touched, quoting the analyzer's buckets:
contains an antd Form -- do not touch until forms unblock):MemoryEditModal.tsx. Its suppression entry staysreached by another page -- REFUSE to edit; Track A owns these):DeprecationBanner.tsx(7 pages),DeleteResourceModal.tsx(23),message_manager.tsx(53),notifications_manager.tsx(53)MemoryTablealready sits on the shared DataTableeslint-suppressions.jsonloses the two migrated files'no-restricted-importsentries, which is the point of the change. It also loses a dead entry formodels-and-endpoints/ModelsAndEndpointsView.tsx; that file no longer exists on this branch, so the entry pointed at nothing and--prune-suppressionsdropped itTwo visible differences a reviewer should expect rather than read as regressions. The "New memory" button is no longer antd blue: a bare
<Button>resolves tobg-primary, which this theme sets to a near-black, matching what the already-migrated prompts page ships. Pinning it back to blue with a utility would hardcode a colour the theme is supposed to own. Separately the scope note under the heading now wraps onto a second line, because the token font metrics are marginally wider than antd's; nothing is clippedFinal Attestation