Feature/dashboard recent requests #872
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ment - Add Recent Requests carousel with status filtering and multi-instance popovers - Add Plex ratings enrichment for movies (RT Critic, RT Audience) and shows (all ratings) - Add RT fresh/rotten icons with 60% threshold logic (copied from Seerr) - Centralize poster URL generation with server/client utilities - Add migration to normalize existing TMDB poster paths in database - Consolidate dashboard SSE handling into unified hook - Add session storage to version check to prevent repeated notifications - Refactor stats header to remove duplicate code with popularity rankings
- Fix tooltip z-index and overflow issues by wrapping content in Portal - Update status labels: pending_approval shows "Awaiting Approval" - Show instance popover for all available items, not just multi-instance - Remove redundant status text from instance popover - Replace native title attributes with proper Tooltip components
Contributor
WalkthroughAdds dashboard recent-requests UI, server API and DB method; introduces TMDB poster-path normalization, builders, migration and tests; surfaces Plex-style ratings in TMDB metadata; updates notification poster resolution; adds carousel responsiveness and multiple presentational style/token updates. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User
participant Dashboard as Dashboard Page
participant RecentReq as RecentRequests Component
participant Hook as useRecentRequests
participant API as /v1/stats/recent-requests
participant DB as Database
User->>Dashboard: Open dashboard
Dashboard->>Hook: mount useDashboardSSE (subscribe)
Dashboard->>RecentReq: render
RecentReq->>Hook: call useRecentRequests()
Hook->>API: GET /v1/stats/recent-requests?limit=...
API->>DB: getRecentRequests(limit, status)
DB-->>API: RecentRequestItem[]
API-->>Hook: { items }
Hook-->>RecentReq: items (render carousel)
Note over Hook,Dashboard: SSE approval events invalidate ['recent-requests'] → Hook refetches
sequenceDiagram
participant MediaCard as MediaCard
participant HookP as usePosterUrl
participant PosterUtil as buildPosterUrl
participant TMDB as TMDB metadata (via GUID)
MediaCard->>HookP: usePosterUrl({thumb, guids, contentType, context})
alt thumb present
HookP->>PosterUtil: buildPosterUrl(thumb, context)
PosterUtil-->>HookP: posterUrl
HookP-->>MediaCard: posterUrl
else thumb missing
HookP->>TMDB: fetch metadata by prioritized GUID
TMDB-->>HookP: { poster_path }
HookP->>PosterUtil: buildPosterUrl(poster_path, context)
PosterUtil-->>HookP: posterUrl
HookP-->>MediaCard: posterUrl
end
MediaCard->>MediaCard: render image
Estimated code review effort🎯 4 (Complex) | ⏱️ ~70 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/client/hooks/useVersionCheck.tsx (1)
31-38: Fix semver comparison crash on non-semver GitHub release tags.
semver.gt()throws TypeError on invalid versions. Sincedata.tag_namefrom GitHub releases can be non-semver (e.g., "nightly", "alpha", custom formats), the code will crash when checking for updates. Also, the toast can produce duplicated "v" prefix if__APP_VERSION__includes it.Proposed patch
// Clean version strings for semver comparison - const currentVersion = __APP_VERSION__.replace(/^v/, '') - const latestVersion = data.tag_name.replace(/^v/, '') + const currentVersion = semver.clean(__APP_VERSION__) ?? __APP_VERSION__.replace(/^v/, '') + const latestVersion = semver.clean(data.tag_name) ?? data.tag_name.replace(/^v/, '') - if (semver.gt(latestVersion, currentVersion)) { + if (semver.valid(latestVersion) && semver.valid(currentVersion) && semver.gt(latestVersion, currentVersion)) { toast( - `A new version (${data.tag_name}) is available. You're running v${__APP_VERSION__}.`, + `A new version (${data.tag_name}) is available. You're running ${__APP_VERSION__}.`, {
🤖 Fix all issues with AI agents
In @src/client/features/dashboard/components/recent-requests.tsx:
- Around line 62-70: The effect registers both 'select' and 'reInit' listeners
on the carousel API (in the useEffect that references api and onSelect) but only
removes the 'select' listener; update the cleanup to remove both by calling
api.off('select', onSelect) and api.off('reInit', onSelect) (guarding with if
(api) as done at the top) so both listeners are unsubscribed when the effect is
torn down or api changes.
In @src/client/features/dashboard/hooks/usePosterUrl.ts:
- Around line 137-140: The effect in usePosterUrl (the useEffect that returns
abortController.abort) incorrectly lists guids.map in its dependency array which
references Array.prototype.map and prevents re-running when guids change;
replace guids.map with a proper dependency such as guids (if the parent
guarantees referential stability) or JSON.stringify(guids) (if order/content
matters) so the effect re-runs when the guids array contents change, and update
the dependency array for the effect that calls fetchPoster URLs accordingly.
🧹 Nitpick comments (13)
src/client/hooks/useVersionCheck.tsx (2)
14-17: GuardsessionStorageaccess + don’t miss setting the session flag on exceptions.
DirectsessionStorage.getItem/setItemcan throw (SSR/blocked storage). Also, the “checked” flag isn’t set iffetch()/json()/semverthrows, so the hook can retry every mount and keep logging.Proposed patch
const VERSION_CHECK_KEY = 'version-check-shown' export const useVersionCheck = (repoOwner: string, repoName: string) => { useEffect(() => { // Only check once per session - if (sessionStorage.getItem(VERSION_CHECK_KEY)) { - return - } + const getSessionFlag = () => { + try { + return sessionStorage.getItem(VERSION_CHECK_KEY) + } catch { + return null + } + } + const setSessionFlag = () => { + try { + sessionStorage.setItem(VERSION_CHECK_KEY, 'true') + } catch { + // ignore (storage unavailable) + } + } + + if (getSessionFlag()) return const checkForUpdates = async () => { try { const response = await fetch( `https://api.github.com/repos/${repoOwner}/${repoName}/releases/latest` ) @@ - // Mark as checked for this session (regardless of whether update exists) - sessionStorage.setItem(VERSION_CHECK_KEY, 'true') } catch (err) { if (err instanceof Error) { console.error(`Error checking for updates: ${err.message}`) } else { console.error('Unknown error checking for updates:', err) } + } finally { + // Mark as checked for this session (regardless of whether update exists) + setSessionFlag() } }Also applies to: 50-52
61-66: Optional: abort/ignore in-flight fetch after unmount.
The timeout cleanup prevents the call from starting, but if it has already started, the fetch can still complete and log/toast after unmount.src/client/lib/poster-url.ts (1)
1-74: Consider sharingbuildPosterUrl()and size constants between client and server.This client-side implementation duplicates the core poster URL building logic and
POSTER_SIZES/TMDB_URL_PATTERNconstants fromsrc/utils/poster-url.ts. While separating client and server code may be intentional for bundle optimization, extracting these shared elements to a common module could reduce maintenance burden if your build system supports cross-environment code sharing.The implementation itself is correct and well-documented.
src/client/styles/globals.css (1)
28-35: Verify CSS custom property naming convention.The new status color tokens use a different pattern than existing color variables:
- Existing:
:rootdefines--main,@themealiases as--color-main- New: Both
:rootand@themeuse--color-status-*While this self-referential pattern is valid in CSS and enables cascading overrides, it diverges from the established convention in this file. Consider whether status colors should follow the existing two-tier naming pattern for consistency.
♻️ Optional refactor to match existing convention
If you prefer consistency with existing patterns:
@theme { - --color-status-pending: var(--color-status-pending); - --color-status-approved: var(--color-status-approved); - --color-status-available: var(--color-status-available); - --color-status-requested: var(--color-status-requested); - --color-status-rejected: var(--color-status-rejected); - --color-status-expired: var(--color-status-expired); - --color-status-auto-approved: var(--color-status-auto-approved); + --color-status-pending: var(--status-pending); + --color-status-approved: var(--status-approved); + --color-status-available: var(--status-available); + --color-status-requested: var(--status-requested); + --color-status-rejected: var(--status-rejected); + --color-status-expired: var(--status-expired); + --color-status-auto-approved: var(--status-auto-approved); } :root { /* Status colors */ - --color-status-pending: #eab308; - --color-status-approved: #22c55e; - --color-status-available: #22c55e; - --color-status-requested: #3b82f6; - --color-status-rejected: #ef4444; - --color-status-expired: #9ca3af; - --color-status-auto-approved: #3b82f6; + --status-pending: #eab308; + --status-approved: #22c55e; + --status-available: #22c55e; + --status-requested: #3b82f6; + --status-rejected: #ef4444; + --status-expired: #9ca3af; + --status-auto-approved: #3b82f6; }Also applies to: 175-182
src/client/features/dashboard/hooks/useDashboardSSE.ts (1)
1-27: LGTM! Clean SSE subscription pattern with proper cleanup.The hook correctly subscribes to approval events, invalidates relevant queries, and cleans up on unmount. The implementation follows React best practices and the documentation clearly indicates it should be called once at the page level.
Optional: queryClient dependency can be omitted
The
queryClientinstance returned byuseQueryClient()is stable and doesn't require inclusion in the dependency array. You can safely omit it:- }, [subscribeToType, queryClient]) + }, [subscribeToType])This is a minor optimization and the current implementation is not incorrect.
src/client/features/dashboard/components/recent-request-card.tsx (3)
58-71: Consider extractingformatTimeAgoto a shared utility.This function duplicates time formatting logic that may exist elsewhere (e.g., in
recent-requests.tsxor other dashboard components). Consider extracting it to a shared utility in@/lib/utilsor a dedicated date formatting module.
177-199: Consider consolidating nestedTooltipProviderwrappers.Each tooltip has its own
TooltipProviderwrapper. While functional, you could wrap the entire card content in a singleTooltipProviderto reduce component nesting and improve performance slightly.♻️ Suggested refactor
+ <TooltipProvider> <Card className={cn('shadow-none', className)}> <CardContent className="p-2.5"> {/* ... content ... */} {/* Content type indicator */} - <TooltipProvider> <Tooltip> <TooltipTrigger asChild> {/* ... */} </TooltipTrigger> <TooltipContent>...</TooltipContent> </Tooltip> - </TooltipProvider> {/* Eye button */} {hasGuids && ( - <TooltipProvider> <Tooltip> {/* ... */} </Tooltip> - </TooltipProvider> )} </CardContent> </Card> + </TooltipProvider>
228-233: Replace nativetitleattribute with Tooltip for consistency.The PR objectives mention replacing native titles with Tooltip components for consistency. Line 230 uses a native
titleattribute on theh3element. Consider using a Tooltip for the truncated title to maintain UI consistency.♻️ Suggested refactor
- <h3 - className="mt-2 line-clamp-1 text-sm font-medium leading-tight" - title={item.title} - > - {item.title} - </h3> + <Tooltip> + <TooltipTrigger asChild> + <h3 className="mt-2 line-clamp-1 text-sm font-medium leading-tight cursor-default"> + {item.title} + </h3> + </TooltipTrigger> + <TooltipContent>{item.title}</TooltipContent> + </Tooltip>src/schemas/dashboard/recent-requests.schema.ts (1)
30-30: Consider adding datetime format validation tocreatedAt.The
createdAtfield is typed asz.string(). Consider adding ISO datetime format validation (e.g.,z.string().datetime()) to catch malformed dates early.♻️ Suggested enhancement
- createdAt: z.string(), + createdAt: z.string().datetime(),src/client/features/dashboard/components/recent-requests.tsx (2)
72-80: Consider documenting the purpose of minimum loading delay.This minimum loading timer works alongside useAppQuery's built-in loading delay. The combination ensures skeletons remain visible for at least 500ms even when data arrives quickly. A brief comment explaining this UX decision would help future maintainers.
201-213: Consider matching skeleton count to the selected limit.The skeleton count is hardcoded to 10, but users can select limits of 10, 20, 30, or 50. This could cause a visual inconsistency where fewer skeletons appear than expected items.
♻️ Proposed fix
{isLoading && (!minLoadingComplete || items.length === 0) - ? Array.from({ length: 10 }, (_, i) => `skeleton-${i}`).map( + ? Array.from({ length: Math.min(limit, 10) }, (_, i) => `skeleton-${i}`).map( (skeletonId) => (src/services/database/methods/dashboard.ts (2)
116-169: Consider documenting the limit multiplier heuristic.The
limit * 3multiplier is used to fetch extra rows to account for multiple instances per watchlist item. This heuristic works well for typical cases but could undercount if items have many instances. A brief comment explaining this tradeoff would help maintainers understand the design decision.
193-257: Consider extracting duplicate row processing logic.The Radarr and Sonarr row processing blocks are nearly identical, differing only in
instanceType. Extracting a helper function would reduce duplication and make future changes easier.♻️ Proposed refactor
// Helper function to process rows into the map function processWatchlistRows( rows: typeof radarrItems, instanceType: 'radarr' | 'sonarr', map: typeof watchlistItemsMap, safeJsonParse: typeof this.safeJsonParse, ) { for (const row of rows) { const existing = map.get(row.id) if (existing) { existing.instances.push({ id: row.instance_id, name: row.instance_name || 'Unknown', status: row.junction_status || 'pending', isPrimary: Boolean(row.is_primary), }) } else { map.set(row.id, { id: row.id, title: row.title, contentType: row.content_type as 'movie' | 'show', guids: safeJsonParse(row.guids, [], 'watchlist_item.guids'), thumb: row.thumb || null, userId: row.user_id, userName: row.user_name || 'Unknown', createdAt: row.created_at, instanceType, instances: [{ id: row.instance_id, name: row.instance_name || 'Unknown', status: row.junction_status || 'pending', isPrimary: Boolean(row.is_primary), }], }) } } } // Usage: processWatchlistRows(radarrItems, 'radarr', watchlistItemsMap, this.safeJsonParse.bind(this)) processWatchlistRows(sonarrItems, 'sonarr', watchlistItemsMap, this.safeJsonParse.bind(this))
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (11)
docs/docs/api/get-recent-requests.api.mdxis excluded by!docs/**docs/docs/api/get-tmdb-metadata-by-guid.api.mdxis excluded by!docs/**docs/docs/api/get-tmdb-movie-metadata.api.mdxis excluded by!docs/**docs/docs/api/get-tmdb-tv-metadata.api.mdxis excluded by!docs/**docs/docs/api/sidebar.tsis excluded by!docs/**docs/static/openapi.jsonis excluded by!docs/**src/client/assets/images/rating-icons/rotten-tomatoes.svgis excluded by!**/*.svg,!src/**/assets/**,!src/**/images/**,!**/*.{png,jpg,jpeg,gif,svg,webp,ico,woff,woff2}src/client/assets/images/rating-icons/rt-aud-fresh.svgis excluded by!**/*.svg,!src/**/assets/**,!src/**/images/**,!**/*.{png,jpg,jpeg,gif,svg,webp,ico,woff,woff2}src/client/assets/images/rating-icons/rt-aud-rotten.svgis excluded by!**/*.svg,!src/**/assets/**,!src/**/images/**,!**/*.{png,jpg,jpeg,gif,svg,webp,ico,woff,woff2}src/client/assets/images/rating-icons/rt-fresh.svgis excluded by!**/*.svg,!src/**/assets/**,!src/**/images/**,!**/*.{png,jpg,jpeg,gif,svg,webp,ico,woff,woff2}src/client/assets/images/rating-icons/rt-rotten.svgis excluded by!**/*.svg,!src/**/assets/**,!src/**/images/**,!**/*.{png,jpg,jpeg,gif,svg,webp,ico,woff,woff2}
📒 Files selected for processing (35)
migrations/migrations/080_20260109_normalize_poster_paths.tssrc/client/components/tmdb-metadata-display.tsxsrc/client/components/ui/tooltip.tsxsrc/client/features/approvals/components/approval-stats-header.tsxsrc/client/features/approvals/components/approval-status-badge.tsxsrc/client/features/dashboard/components/media-card.tsxsrc/client/features/dashboard/components/popularity-rankings.tsxsrc/client/features/dashboard/components/recent-request-card.tsxsrc/client/features/dashboard/components/recent-requests.tsxsrc/client/features/dashboard/components/stats-header.tsxsrc/client/features/dashboard/hooks/useDashboardSSE.tssrc/client/features/dashboard/hooks/usePosterUrl.tssrc/client/features/dashboard/hooks/useRecentRequests.tssrc/client/features/dashboard/index.tsxsrc/client/hooks/useVersionCheck.tsxsrc/client/lib/poster-url.tssrc/client/styles/globals.csssrc/routes/v1/stats/recent-requests.tssrc/schemas/dashboard/recent-requests.schema.tssrc/schemas/tmdb/tmdb.schema.tssrc/services/database.service.tssrc/services/database/methods/dashboard.tssrc/services/database/types/dashboard-methods.tssrc/services/notifications/discord-bot/commands/approval/data.tssrc/services/notifications/orchestration/approval.tssrc/services/notifications/orchestration/media-available.tssrc/services/notifications/orchestration/watchlist-added.tssrc/services/plex-watchlist/cache/rss-feed-cache.tssrc/services/plex-watchlist/enrichment/rss-item-enricher.tssrc/services/plex-watchlist/enrichment/single-item.tssrc/services/plex-watchlist/fetching/rss-fetcher.tssrc/services/plex-watchlist/fetching/watchlist-fetcher.tssrc/services/tmdb.service.tssrc/utils/poster-url.tstest/unit/utils/poster-url.test.ts
🧰 Additional context used
🧠 Learnings (30)
📓 Common learnings
Learnt from: jamcalli
Repo: jamcalli/Pulsarr PR: 583
File: src/services/content-router.service.ts:2249-2256
Timestamp: 2025-09-23T04:36:56.478Z
Learning: In PR #583, the auto_approved status has been comprehensively implemented across the entire Pulsarr codebase including database migrations, TypeScript types, Zod schemas, UI components, and database methods. The createAutoApprovalRecord method in src/services/content-router.service.ts correctly uses this well-defined status that exists throughout the system.
Learnt from: jamcalli
Repo: jamcalli/Pulsarr PR: 434
File: src/types/radarr.types.ts:16-16
Timestamp: 2025-08-07T06:30:00.080Z
Learning: In the Pulsarr codebase, when jamcalli identifies technical debt that involves type inconsistencies (like RadarrMovie using number[] vs RadarrPost using string[] for tags), they prefer to defer complex fixes to separate PRs if downstream conversion is already handling the discrepancy and functionality is working correctly.
📚 Learning: 2025-09-24T08:20:26.185Z
Learnt from: jamcalli
Repo: jamcalli/Pulsarr PR: 594
File: src/services/watchlist-workflow.service.ts:1518-1520
Timestamp: 2025-09-24T08:20:26.185Z
Learning: The database methods getAllShowWatchlistItems() and getAllMovieWatchlistItems() in the Pulsarr codebase return Promise<TokenWatchlistItem[]>, not Promise<WatchlistItem[]>. TokenWatchlistItem extends the base Item/WatchlistItem interface with additional properties like id: string.
Applied to files:
src/services/notifications/orchestration/media-available.tssrc/services/notifications/discord-bot/commands/approval/data.tssrc/services/database/methods/dashboard.tssrc/services/notifications/orchestration/approval.tssrc/services/plex-watchlist/fetching/watchlist-fetcher.tssrc/services/tmdb.service.ts
📚 Learning: 2025-04-25T06:50:49.751Z
Learnt from: jamcalli
Repo: jamcalli/Pulsarr PR: 110
File: src/services/watchlist-workflow.service.ts:345-349
Timestamp: 2025-04-25T06:50:49.751Z
Learning: In the watchlist workflow service, the `createItemMap` function intentionally uses only the first GUID from `parseGuids(item.guids)` as the map key. This is by design because the change detection logic relies on a 1:1 mapping between items and their primary GUIDs. The `parseGuids()` function handles GUID normalization and prioritization, ensuring consistent primary GUID selection that matches how Plex, Sonarr, and Radarr handle primary identifiers.
Applied to files:
src/services/notifications/orchestration/media-available.ts
📚 Learning: 2025-09-04T07:24:01.359Z
Learnt from: jamcalli
Repo: jamcalli/Pulsarr PR: 545
File: src/services/content-router.service.ts:1013-1046
Timestamp: 2025-09-04T07:24:01.359Z
Learning: In the content router service IMDb enrichment code, when extracting IMDb IDs from both item.guids and metadata fallback, always call getRating(item.guids) rather than constructing artificial IMDb GUIDs. This preserves data integrity - items without genuine IMDb GUIDs should return null from getRating(), as ~2% of content legitimately lacks IMDb data. Don't fabricate missing IMDb GUIDs even when metadata contains an IMDb ID.
Applied to files:
src/services/notifications/orchestration/media-available.tssrc/client/components/tmdb-metadata-display.tsx
📚 Learning: 2025-12-28T09:36:36.861Z
Learnt from: jamcalli
Repo: jamcalli/Pulsarr PR: 839
File: src/client/features/dashboard/components/media-card.tsx:82-82
Timestamp: 2025-12-28T09:36:36.861Z
Learning: In the Pulsarr watchlist system, users are unique per media item—a user can only watchlist an item once. Therefore, the item.users array in ContentStat cannot contain duplicate usernames, making it safe to use the username string directly as a React key when mapping over users for a specific media item.
Applied to files:
src/services/notifications/orchestration/media-available.ts
📚 Learning: 2025-12-29T18:26:34.049Z
Learnt from: jamcalli
Repo: jamcalli/Pulsarr PR: 842
File: src/services/database/methods/analytics.ts:197-200
Timestamp: 2025-12-29T18:26:34.049Z
Learning: In SQLite usage within TypeScript code, ensure JSON_GROUP_ARRAY(DISTINCT col) is used for distinct aggregation of a single column. Do not use GROUP_CONCAT(DISTINCT col, delimiter) because the 2-argument form with DISTINCT and a delimiter is invalid. When reviewing analytics-related SQL strings in src/services/database/methods/analytics.ts, prefer JSON_GROUP_ARRAY(DISTINCT <column>) for distinct arrays and avoid the invalid GROUP_CONCAT(DISTINCT <column>, <delimiter>) pattern.
Applied to files:
src/services/notifications/orchestration/media-available.tssrc/services/plex-watchlist/cache/rss-feed-cache.tssrc/client/lib/poster-url.tssrc/routes/v1/stats/recent-requests.tssrc/services/notifications/orchestration/watchlist-added.tssrc/utils/poster-url.tssrc/services/notifications/discord-bot/commands/approval/data.tssrc/services/plex-watchlist/enrichment/single-item.tssrc/schemas/dashboard/recent-requests.schema.tssrc/services/database/methods/dashboard.tssrc/client/features/dashboard/hooks/useRecentRequests.tssrc/services/notifications/orchestration/approval.tssrc/services/database/types/dashboard-methods.tsmigrations/migrations/080_20260109_normalize_poster_paths.tssrc/client/features/dashboard/hooks/useDashboardSSE.tssrc/services/plex-watchlist/enrichment/rss-item-enricher.tssrc/services/plex-watchlist/fetching/rss-fetcher.tstest/unit/utils/poster-url.test.tssrc/schemas/tmdb/tmdb.schema.tssrc/client/features/dashboard/hooks/usePosterUrl.tssrc/services/database.service.tssrc/services/plex-watchlist/fetching/watchlist-fetcher.tssrc/services/tmdb.service.ts
📚 Learning: 2025-12-02T09:00:20.744Z
Learnt from: jamcalli
Repo: jamcalli/Pulsarr PR: 784
File: src/services/plex-watchlist/sync/item-categorizer.ts:29-46
Timestamp: 2025-12-02T09:00:20.744Z
Learning: In src/services/plex-watchlist/sync/item-categorizer.ts, the createWatchlistItem function intentionally uses `key: item.id` (not `templateItem.key`) when creating linked watchlist items for different users. This ensures consistent storage of the Plex ratingKey regardless of how the lookup was performed.
Applied to files:
src/services/plex-watchlist/cache/rss-feed-cache.tssrc/services/plex-watchlist/enrichment/single-item.tssrc/services/plex-watchlist/enrichment/rss-item-enricher.tssrc/services/plex-watchlist/fetching/rss-fetcher.tssrc/services/plex-watchlist/fetching/watchlist-fetcher.tssrc/services/tmdb.service.ts
📚 Learning: 2025-08-08T06:55:52.201Z
Learnt from: jamcalli
Repo: jamcalli/Pulsarr PR: 437
File: src/services/plex-label-sync.service.ts:0-0
Timestamp: 2025-08-08T06:55:52.201Z
Learning: In src/services/plex-label-sync.service.ts, prefer simple, case-insensitive startsWith-based prefix checks for tag and removed-tag detection (matching user-tag.service). Keep any regex-based validation at the API schema layer, as services can trust validated config values. This avoids variable-based regex in services and maintains consistency across modules.
Applied to files:
src/services/plex-watchlist/cache/rss-feed-cache.tssrc/services/plex-watchlist/fetching/rss-fetcher.ts
📚 Learning: 2025-05-28T22:04:22.212Z
Learnt from: jamcalli
Repo: jamcalli/Pulsarr PR: 224
File: src/routes/v1/tautulli/test.ts:29-35
Timestamp: 2025-05-28T22:04:22.212Z
Learning: In the Pulsarr codebase, RSS feeds (selfRss and friendsRss) are Plex API endpoints that are only available to Plex Pass subscribers, not local server-generated feeds. Checking for the existence of these RSS feed URLs in the configuration is used consistently throughout the codebase as a reliable method to validate Plex Pass subscription status.
Applied to files:
src/services/plex-watchlist/cache/rss-feed-cache.ts
📚 Learning: 2025-10-23T21:17:44.151Z
Learnt from: jamcalli
Repo: jamcalli/Pulsarr PR: 668
File: src/utils/plex/rss.ts:26-37
Timestamp: 2025-10-23T21:17:44.151Z
Learning: In the Pulsarr codebase, RSS feed URLs returned by the Plex API (from getRssFromPlexToken) are public URLs that do not contain tokens or other sensitive information. These URLs (e.g., https://rss.plex.tv/feed/watchlist/abc123) can be safely logged without redaction. The Plex token is only used to authenticate the initial request to generate the RSS URL, but the resulting feed URL itself is public.
Applied to files:
src/services/plex-watchlist/cache/rss-feed-cache.ts
📚 Learning: 2025-06-29T19:45:44.866Z
Learnt from: jamcalli
Repo: jamcalli/Pulsarr PR: 0
File: :0-0
Timestamp: 2025-06-29T19:45:44.866Z
Learning: When a PR involves Tailwind v3 to v4 auto-migration, most CSS class changes (like text-text → text-foreground, bg-bw → bg-secondary-background) are automated migration artifacts and don't require detailed review comments. Focus should be on architectural and functional changes rather than styling updates.
Applied to files:
src/client/features/approvals/components/approval-stats-header.tsxsrc/client/styles/globals.csssrc/client/features/approvals/components/approval-status-badge.tsx
📚 Learning: 2025-09-23T04:36:56.478Z
Learnt from: jamcalli
Repo: jamcalli/Pulsarr PR: 583
File: src/services/content-router.service.ts:2249-2256
Timestamp: 2025-09-23T04:36:56.478Z
Learning: In PR #583, the auto_approved status has been comprehensively implemented across the entire Pulsarr codebase including database migrations, TypeScript types, Zod schemas, UI components, and database methods. The createAutoApprovalRecord method in src/services/content-router.service.ts correctly uses this well-defined status that exists throughout the system.
Applied to files:
src/client/features/approvals/components/approval-stats-header.tsxsrc/services/database/methods/dashboard.tssrc/client/styles/globals.csssrc/client/features/approvals/components/approval-status-badge.tsx
📚 Learning: 2025-04-02T16:38:58.001Z
Learnt from: jamcalli
Repo: jamcalli/Pulsarr PR: 76
File: src/routes/v1/plex/configure-notifications.ts:14-0
Timestamp: 2025-04-02T16:38:58.001Z
Learning: When using fastify-type-provider-zod in Fastify routes, schema validation occurs automatically before the handler is called, making explicit validation in the handler code redundant for fields already validated in the schema.
Applied to files:
src/routes/v1/stats/recent-requests.ts
📚 Learning: 2025-06-11T08:13:10.139Z
Learnt from: jamcalli
Repo: jamcalli/Pulsarr PR: 281
File: src/services/user-tag.service.ts:226-230
Timestamp: 2025-06-11T08:13:10.139Z
Learning: Prefer importing the canonical `User` type (from `config.types.ts`), which already includes `can_sync: boolean`, instead of redeclaring a local interface in `user-tag.service.ts`.
Applied to files:
src/services/notifications/discord-bot/commands/approval/data.ts
📚 Learning: 2025-06-02T06:24:50.446Z
Learnt from: jamcalli
Repo: jamcalli/Pulsarr PR: 238
File: src/types/config.types.ts:141-146
Timestamp: 2025-06-02T06:24:50.446Z
Learning: The Pulsarr codebase follows a consistent pattern for complex configuration data: store as JSON strings, parse without validation in service layers (env.ts, database.service.ts), and validate at the API layer through Zod schemas in ConfigSchema. This applies to sonarrTags, radarrTags, plexTokens, and plexSessionMonitoring.
Applied to files:
src/schemas/dashboard/recent-requests.schema.tssrc/schemas/tmdb/tmdb.schema.ts
📚 Learning: 2025-04-24T21:05:18.033Z
Learnt from: jamcalli
Repo: jamcalli/Pulsarr PR: 110
File: src/services/database.service.ts:0-0
Timestamp: 2025-04-24T21:05:18.033Z
Learning: The getDetailedStatusTransitionMetrics method in database.service.ts should use JavaScript-based quartile calculations and outlier filtering instead of SQL PERCENTILE_CONT, as SQLite (used via better-sqlite3) doesn't support the PERCENTILE_CONT window function.
Applied to files:
src/services/database/methods/dashboard.ts
📚 Learning: 2025-04-22T22:36:01.170Z
Learnt from: jamcalli
Repo: jamcalli/Pulsarr PR: 110
File: migrations/migrations/013_20250414_update_router_rules_format.ts:164-173
Timestamp: 2025-04-22T22:36:01.170Z
Learning: In migrations/migrations/013_20250414_update_router_rules_format.ts, the apparent inconsistency between `up` and `down` migrations for language rules is intentional by design. The `up` migration converts from 'originalLanguage' field to 'language' to match evaluator expectations, while the `down` migration reverses this transformation. This pattern is the correct approach for handling field name changes in migrations.
Applied to files:
migrations/migrations/080_20260109_normalize_poster_paths.ts
📚 Learning: 2025-08-08T08:05:27.194Z
Learnt from: jamcalli
Repo: jamcalli/Pulsarr PR: 438
File: package.json:18-18
Timestamp: 2025-08-08T08:05:27.194Z
Learning: Pulsarr repo preference: Keep migrations/rollback.ts (TypeScript, Knex) simple without adding Knex CLI-style flags like --step or --env. Do not suggest CLI parity for this script unless the maintainer requests it.
Applied to files:
migrations/migrations/080_20260109_normalize_poster_paths.ts
📚 Learning: 2026-01-01T09:40:57.305Z
Learnt from: jamcalli
Repo: jamcalli/Pulsarr PR: 844
File: migrations/migrations/076_20251230_add_plex_ratings_columns.ts:3-13
Timestamp: 2026-01-01T09:40:57.305Z
Learning: In Pulsarr, database storage uses a 0-10 scale for RT ratings in watchlist_items.rt_critic_rating and rt_audience_rating. The UI displays 0-100%, but the internal scale remains 0-10. Migration docs for this repository should describe the storage scale (0-10) rather than the UI scale, ensuring migrations reflect how data is actually stored and compared (convertToInternalScale converts 0-100 input back to 0-10 for DB comparisons).
Applied to files:
migrations/migrations/080_20260109_normalize_poster_paths.ts
📚 Learning: 2026-01-01T09:41:03.037Z
Learnt from: jamcalli
Repo: jamcalli/Pulsarr PR: 844
File: migrations/migrations/076_20251230_add_plex_ratings_columns.ts:3-13
Timestamp: 2026-01-01T09:41:03.037Z
Learning: In Pulsarr's rating system, Rotten Tomatoes ratings flow through multiple layers with scale conversions: Plex API returns RT ratings normalized to 0-10, the database (watchlist_items table, columns rt_critic_rating and rt_audience_rating) stores these 0-10 values as received, the UI (condition-input.tsx) displays them as 0-100% for user-friendliness, and the ratings-evaluator converts user input (0-100) back to 0-10 scale for database comparisons using convertToInternalScale. Migration documentation should reflect the database storage scale (0-10), not the UI presentation scale.
Applied to files:
src/services/plex-watchlist/enrichment/rss-item-enricher.tssrc/schemas/tmdb/tmdb.schema.tssrc/client/components/tmdb-metadata-display.tsxsrc/services/tmdb.service.ts
📚 Learning: 2025-08-08T17:48:13.959Z
Learnt from: jamcalli
Repo: jamcalli/Pulsarr PR: 438
File: src/utils/plex-server.ts:1777-1852
Timestamp: 2025-08-08T17:48:13.959Z
Learning: In src/utils/plex-server.ts, the label update implementation correctly uses `label[].tag.tag` for setting labels (repeated for each label) and `label[].tag.tag-` for clearing all labels, along with `label.locked=1` to lock the field. This follows established patterns from the open source Plex API community and should not be changed to indexed parameters.
Applied to files:
src/schemas/tmdb/tmdb.schema.ts
📚 Learning: 2025-08-08T19:59:22.376Z
Learnt from: jamcalli
Repo: jamcalli/Pulsarr PR: 438
File: src/schemas/notifications/webhook.schema.ts:29-29
Timestamp: 2025-08-08T19:59:22.376Z
Learning: In Pulsarr webhook schemas (src/schemas/notifications/webhook.schema.ts), the tags field in both RadarrWebhookPayloadSchema and SonarrWebhookPayloadSchema should be defined as z.array(z.string()).optional() because incoming webhooks from Radarr and Sonarr contain tag names as strings, not numeric IDs, in their payloads.
Applied to files:
src/schemas/tmdb/tmdb.schema.ts
📚 Learning: 2025-08-15T05:57:29.728Z
Learnt from: jamcalli
Repo: jamcalli/Pulsarr PR: 467
File: src/client/features/plex/hooks/usePlexUser.ts:5-6
Timestamp: 2025-08-15T05:57:29.728Z
Learning: In the Pulsarr project, Biome auto-converts schema imports to type imports (e.g., `import type { plexUserSchema }`). Modern TypeScript correctly handles `typeof` operations on type-imported schemas (e.g., `z.input<typeof plexUserSchema>`), so these imports should not be flagged or suggested to be changed to value imports.
Applied to files:
src/schemas/tmdb/tmdb.schema.ts
📚 Learning: 2025-06-07T19:07:26.912Z
Learnt from: jamcalli
Repo: jamcalli/Pulsarr PR: 261
File: src/plugins/custom/database.ts:14-14
Timestamp: 2025-06-07T19:07:26.912Z
Learning: The DatabaseService.create factory method exists in src/services/database.service.ts and is properly implemented with the correct signature. It's used correctly in src/plugins/custom/database.ts for asynchronous PostgreSQL initialization.
Applied to files:
src/services/database.service.ts
📚 Learning: 2025-06-14T08:45:28.216Z
Learnt from: jamcalli
Repo: jamcalli/Pulsarr PR: 294
File: src/services/database/types/junction-methods.ts:3-4
Timestamp: 2025-06-14T08:45:28.216Z
Learning: In this ESM project, module augmentations should use the .js extension (e.g., 'declare module 'services/database.service.js'') to match the import/export patterns used throughout the codebase, even for TypeScript files that get compiled.
Applied to files:
src/services/database.service.ts
📚 Learning: 2025-06-07T18:55:15.887Z
Learnt from: jamcalli
Repo: jamcalli/Pulsarr PR: 261
File: src/services/database.service.ts:250-256
Timestamp: 2025-06-07T18:55:15.887Z
Learning: The getDateDiffSQL method in src/services/database.service.ts is safely implemented as a private internal utility that only receives hardcoded column names as parameters, not user input, making SQL injection concerns not applicable to its actual usage patterns.
Applied to files:
src/services/database.service.ts
📚 Learning: 2025-09-04T07:24:17.397Z
Learnt from: jamcalli
Repo: jamcalli/Pulsarr PR: 545
File: src/services/content-router.service.ts:1154-1182
Timestamp: 2025-09-04T07:24:17.397Z
Learning: For TV shows in the content router service, IMDB rating fetching intentionally does NOT use fallback GUID construction from series metadata (unlike movies). This is because ~2% of TV content legitimately lacks IMDB data, and the system should return null rather than fabricate missing IMDB GUIDs. The current implementation correctly handles both items with existing IMDB GUIDs and items without IMDB data.
Applied to files:
src/client/components/tmdb-metadata-display.tsx
📚 Learning: 2025-09-04T07:55:59.450Z
Learnt from: jamcalli
Repo: jamcalli/Pulsarr PR: 545
File: src/client/features/content-router/components/imdb-rating-input.tsx:95-105
Timestamp: 2025-09-04T07:55:59.450Z
Learning: In IMDB rating input components, compound values like { rating: null, votes: N } are valid and represent "any rating, minimum N votes" filtering - a legitimate scenario where users want to filter by vote count without constraining the rating. The router backend correctly handles null rating values in compound objects as "no rating constraint".
Applied to files:
src/client/components/tmdb-metadata-display.tsx
📚 Learning: 2025-04-10T06:12:28.015Z
Learnt from: jamcalli
Repo: jamcalli/Pulsarr PR: 85
File: src/types/content-lookup.types.ts:251-261
Timestamp: 2025-04-10T06:12:28.015Z
Learning: Radarr API responses will always have a `tmdbId` property and never have a `tvdbId` property, while Sonarr API responses will always have a `tvdbId` property. This distinct separation makes these properties reliable discriminators for type guards.
Applied to files:
src/services/tmdb.service.ts
📚 Learning: 2025-11-23T09:23:18.939Z
Learnt from: jamcalli
Repo: jamcalli/Pulsarr PR: 743
File: src/client/features/content-router/components/condition-input.tsx:462-479
Timestamp: 2025-11-23T09:23:18.939Z
Learning: In the Pulsarr codebase, the StreamingProviderMultiSelect component uses useConfigStore() to reactively watch for tmdbRegion changes. When TmdbRegionSelector updates the region via updateConfig({ tmdbRegion }), StreamingProviderMultiSelect automatically refetches providers for the new region. No explicit onRegionChange callback is needed between these components.
Applied to files:
src/services/tmdb.service.ts
🧬 Code graph analysis (26)
src/services/notifications/orchestration/media-available.ts (2)
src/client/lib/poster-url.ts (1)
buildPosterUrl(53-74)src/utils/poster-url.ts (1)
buildPosterUrl(93-114)
src/services/plex-watchlist/cache/rss-feed-cache.ts (1)
src/utils/poster-url.ts (1)
normalizePosterPath(56-68)
src/client/lib/poster-url.ts (1)
src/utils/poster-url.ts (2)
PosterContext(11-11)buildPosterUrl(93-114)
src/routes/v1/stats/recent-requests.ts (2)
src/schemas/dashboard/recent-requests.schema.ts (2)
RecentRequestsQuerySchema(36-39)RecentRequestsResponseSchema(42-45)src/utils/route-errors.ts (1)
logRouteError(51-96)
src/services/notifications/orchestration/watchlist-added.ts (2)
src/client/lib/poster-url.ts (1)
buildPosterUrl(53-74)src/utils/poster-url.ts (1)
buildPosterUrl(93-114)
src/utils/poster-url.ts (1)
src/client/lib/poster-url.ts (2)
PosterContext(10-10)buildPosterUrl(53-74)
src/services/notifications/discord-bot/commands/approval/data.ts (2)
src/client/lib/poster-url.ts (1)
buildPosterUrl(53-74)src/utils/poster-url.ts (1)
buildPosterUrl(93-114)
src/client/features/dashboard/components/media-card.tsx (2)
src/client/features/dashboard/hooks/usePosterUrl.ts (1)
usePosterUrl(47-153)src/client/lib/utils.ts (1)
cn(10-12)
src/services/plex-watchlist/enrichment/single-item.ts (1)
src/utils/poster-url.ts (1)
normalizePosterPath(56-68)
src/client/features/dashboard/components/recent-requests.tsx (2)
src/client/features/dashboard/hooks/useRecentRequests.ts (4)
useRecentRequests(53-97)STATUS_FILTER_OPTIONS(14-19)LIMIT_PRESETS(21-21)getLimitLabel(24-26)src/client/lib/utils.ts (1)
cn(10-12)
src/services/database/methods/dashboard.ts (1)
src/schemas/dashboard/recent-requests.schema.ts (1)
RecentRequestItem(50-50)
src/client/features/dashboard/components/recent-request-card.tsx (5)
src/schemas/dashboard/recent-requests.schema.ts (1)
RecentRequestItem(50-50)src/client/features/dashboard/hooks/usePosterUrl.ts (1)
usePosterUrl(47-153)src/schemas/stats/stats.schema.ts (1)
ContentStat(162-162)src/client/lib/utils.ts (1)
cn(10-12)src/client/components/content-detail-modal.tsx (1)
ContentDetailModal(31-97)
src/client/features/dashboard/hooks/useRecentRequests.ts (3)
src/schemas/dashboard/recent-requests.schema.ts (4)
RecentRequestStatus(48-48)RecentRequestItem(50-50)RecentRequestsResponse(52-54)RecentRequestsResponseSchema(42-45)src/client/lib/useAppQuery.ts (1)
useAppQuery(28-72)src/client/lib/apiClient.ts (1)
apiClient(93-122)
src/client/features/dashboard/index.tsx (4)
src/client/features/dashboard/hooks/useDashboardSSE.ts (1)
useDashboardSSE(14-27)src/client/features/dashboard/components/stats-header.tsx (1)
StatsHeader(6-15)src/client/features/dashboard/components/recent-requests.tsx (1)
RecentRequests(29-232)src/client/features/dashboard/components/popularity-rankings.tsx (1)
PopularityRankings(23-113)
src/services/notifications/orchestration/approval.ts (2)
src/client/lib/poster-url.ts (1)
buildPosterUrl(53-74)src/utils/poster-url.ts (1)
buildPosterUrl(93-114)
src/services/database/types/dashboard-methods.ts (1)
src/schemas/dashboard/recent-requests.schema.ts (2)
RecentRequestStatus(48-48)RecentRequestItem(50-50)
migrations/migrations/080_20260109_normalize_poster_paths.ts (1)
src/services/database.service.ts (1)
isPostgreSQL(192-194)
src/client/features/dashboard/components/stats-header.tsx (1)
src/client/components/ui/workflow-status-badge.tsx (1)
WatchlistStatusBadge(30-237)
src/client/features/dashboard/hooks/useDashboardSSE.ts (2)
src/client/lib/queryClient.ts (1)
queryClient(13-26)src/client/stores/progressStore.ts (1)
useProgressStore(27-217)
src/services/plex-watchlist/enrichment/rss-item-enricher.ts (1)
src/utils/poster-url.ts (1)
normalizePosterPath(56-68)
src/services/plex-watchlist/fetching/rss-fetcher.ts (1)
src/utils/poster-url.ts (1)
normalizePosterPath(56-68)
test/unit/utils/poster-url.test.ts (2)
src/utils/poster-url.ts (2)
normalizePosterPath(56-68)buildPosterUrl(93-114)src/client/lib/poster-url.ts (1)
buildPosterUrl(53-74)
src/client/features/dashboard/components/popularity-rankings.tsx (2)
src/client/features/dashboard/hooks/useDashboardStats.ts (1)
useDashboardStats(58-96)src/client/features/dashboard/hooks/useRecentRequests.ts (2)
LIMIT_PRESETS(21-21)getLimitLabel(24-26)
src/client/features/dashboard/hooks/usePosterUrl.ts (3)
src/client/lib/poster-url.ts (2)
PosterContext(10-10)buildPosterUrl(53-74)src/utils/poster-url.ts (2)
PosterContext(11-11)buildPosterUrl(93-114)src/client/lib/api.ts (1)
api(6-21)
src/services/plex-watchlist/fetching/watchlist-fetcher.ts (1)
src/utils/poster-url.ts (1)
normalizePosterPath(56-68)
src/services/tmdb.service.ts (1)
src/schemas/tmdb/tmdb.schema.ts (1)
PlexRatings(270-270)
- Add semver.valid() checks before calling semver.gt() to prevent TypeError - Use semver.clean() for more robust version normalization - Gracefully skip update notification when GitHub tag is non-semver
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @src/client/features/dashboard/components/recent-requests.tsx:
- Around line 193-198: In the RecentRequests JSX
(src/client/features/dashboard/components/recent-requests.tsx) remove the
conflicting text color utility by deleting "text-foreground" from the
placeholder span and keep only "text-muted-foreground" on the <span> that
renders "No recent requests" so the muted color is applied consistently.
🧹 Nitpick comments (2)
src/client/features/dashboard/components/recent-request-card.tsx (1)
58-71: Consider adding input validation for date parsing.The function works well for typical cases. However, invalid
dateStringvalues would produceNaNcomparisons that fall through totoLocaleDateString()on an invalid Date. This is a minor edge case if upstream data is validated.Optional: Add validation for robustness
function formatTimeAgo(dateString: string): string { const date = new Date(dateString) + if (Number.isNaN(date.getTime())) return 'Unknown' const now = new Date()src/client/hooks/useVersionCheck.tsx (1)
42-53: Use the cleanedlatestVersionin the toast message for consistent version formatting.The toast message uses
data.tag_name(raw, unprocessed) for the new version butv${currentVersion}(cleaned) for the current version. This creates inconsistent formatting—for example, if the GitHub tag is "1.2.3", the message would read "A new version (1.2.3) is available. You're running v1.2.3."Since
latestVersionis already computed and cleaned at line 34-35, use it instead ofdata.tag_namein the toast message for consistency.♻️ Suggested improvement
toast( - `A new version (${data.tag_name}) is available. You're running v${currentVersion}.`, + `A new version (v${latestVersion}) is available. You're running v${currentVersion}.`, { id: 'version-update-notification', duration: 8000, action: { label: 'View Release', onClick: () => window.open(data.html_url, '_blank', 'noopener,noreferrer'), }, } )
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/client/features/dashboard/components/recent-request-card.tsxsrc/client/features/dashboard/components/recent-requests.tsxsrc/client/features/dashboard/components/watchlist-carousel.tsxsrc/client/features/dashboard/hooks/usePosterUrl.tssrc/client/hooks/useVersionCheck.tsxsrc/client/styles/globals.css
🚧 Files skipped from review as they are similar to previous changes (2)
- src/client/features/dashboard/hooks/usePosterUrl.ts
- src/client/styles/globals.css
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: jamcalli
Repo: jamcalli/Pulsarr PR: 583
File: src/services/content-router.service.ts:2249-2256
Timestamp: 2025-09-23T04:36:56.478Z
Learning: In PR #583, the auto_approved status has been comprehensively implemented across the entire Pulsarr codebase including database migrations, TypeScript types, Zod schemas, UI components, and database methods. The createAutoApprovalRecord method in src/services/content-router.service.ts correctly uses this well-defined status that exists throughout the system.
📚 Learning: 2025-12-21T21:49:10.503Z
Learnt from: jamcalli
Repo: jamcalli/Pulsarr PR: 825
File: src/client/features/notifications/components/webhooks/webhook-endpoint-modal.tsx:65-68
Timestamp: 2025-12-21T21:49:10.503Z
Learning: In React components using Credenza (which wraps Radix Dialog/Drawer with DialogPortal and no forceMount prop), the modal content is fully unmounted when open={false} and remounts fresh on each open. This means useState initializers execute fresh each time the modal opens, so state initialization from form values or props works correctly without needing useEffect synchronization. Don't flag useState initialization as a potential stale state issue in Credenza modals.
Applied to files:
src/client/features/dashboard/components/watchlist-carousel.tsx
🧬 Code graph analysis (1)
src/client/features/dashboard/components/recent-request-card.tsx (10)
src/schemas/dashboard/recent-requests.schema.ts (1)
RecentRequestItem(50-50)src/client/features/dashboard/hooks/usePosterUrl.ts (1)
usePosterUrl(47-153)src/schemas/stats/stats.schema.ts (1)
ContentStat(162-162)src/client/components/ui/badge.tsx (1)
Badge(33-33)src/client/lib/utils.ts (1)
cn(10-12)src/client/components/ui/tooltip.tsx (4)
TooltipProvider(176-176)Tooltip(176-176)TooltipTrigger(176-176)TooltipContent(176-176)src/client/components/ui/card.tsx (2)
Card(79-79)CardContent(79-79)src/client/components/ui/popover.tsx (1)
Popover(30-30)src/client/components/ui/button.tsx (1)
Button(67-67)src/client/components/content-detail-modal.tsx (1)
ContentDetailModal(31-97)
🔇 Additional comments (21)
src/client/features/dashboard/components/watchlist-carousel.tsx (1)
77-86: LGTM! Proper event listener cleanup implemented.The addition of the 'reInit' event listener cleanup correctly prevents a memory leak. All registered listeners are now properly removed in the cleanup function.
src/client/features/dashboard/components/recent-request-card.tsx (7)
1-27: LGTM!Imports are well-organized, separating type imports from value imports. The
RecentRequestCardPropsinterface is clean and appropriately minimal.
29-56: LGTM!Good use of centralized configuration objects for status styling and icons. The
as constassertion ensures type safety for the variant property.
78-96: LGTM!Good use of derived boolean flags for conditional rendering. The
usePosterUrlhook is properly configured with all relevant parameters, and theenabledflag correctly gates the TMDB fetch when GUIDs are unavailable.
98-119: LGTM!The
ContentStatconstruction correctly maps item properties. ExtractingStatusBadgeContentas a variable avoids JSX duplication between the popover and non-popover branches.
121-175: LGTM!Good implementation of the conditional instance popover. The key generation using
${instance.instanceType}-${instance.id}ensures uniqueness across different instance types.
177-221: LGTM!Proper event handling with
preventDefault()andstopPropagation()on the eye button click. Thesr-onlyspan provides good accessibility for screen readers alongside thearia-label.
223-246: LGTM!The modal is correctly rendered outside the
TooltipProviderto avoid z-index/stacking context issues, and is properly gated byhasGuids.src/client/features/dashboard/components/recent-requests.tsx (8)
1-23: LGTM!Imports are well-organized with proper separation of external libraries, internal UI components, and feature-specific hooks.
29-46: LGTM!The media query breakpoints align with the existing
WatchlistCarouselpattern, ensuring consistent responsive behavior across dashboard components.
62-71: LGTM!Proper cleanup of both
selectandreInitevent listeners. This aligns with the commit message fix for missingreInitcleanup.
73-81: LGTM!The 500ms minimum loading delay prevents skeleton flicker on fast responses, improving perceived UX.
85-116: LGTM!The responsive class logic is well-documented with comments explaining the relationship to
WatchlistCarousel. All breakpoint variables are correctly included in the dependency array.
118-134: LGTM!Empty dependency arrays are correct since
STATUS_FILTER_OPTIONSandLIMIT_PRESETSare module-level constants that won't change.
136-160: LGTM!Clean header layout with filters and navigation link. The
disabled={isLoading}on selects prevents user interaction during data fetching.
200-227: LGTM!Good handling of loading vs data states. The skeleton count of 10 provides reasonable visual continuity, and the item key
${item.source}-${item.id}ensures uniqueness across different request sources.src/client/hooks/useVersionCheck.tsx (5)
15-17: LGTM! Session-scoped check prevents redundant API calls.The early return using
sessionStorageeffectively prevents multiple version checks within the same browser session, which is a good optimization for the GitHub API rate limits and user experience.
32-35: LGTM! Good defensive version normalization.The use of
semver.clean()with a fallback that strips the 'v' prefix is appropriate. Ifsemver.clean()returns null for invalid versions, the fallback ensures a best-effort comparison, and the subsequentsemver.valid()checks on lines 38-39 will catch any remaining invalid formats.
37-41: Excellent defensive programming for semver comparison.The validation guards ensure that
semver.gt()only receives valid semver strings, preventing runtime errors from malformed version tags. This directly addresses the PR objective to guard semver comparisons and skip notifications for non-semver GitHub tags.
56-57: Verify session flag placement is intentional.The
sessionStorageflag is set inside thetryblock after all operations complete successfully. This means:
- ✅ Flag is set when check succeeds (whether or not an update exists)
- ❌ Flag is NOT set when errors occur (network failures, API errors)
As a result, failed checks will retry on every component mount until they succeed. This could lead to repeated API calls if GitHub is temporarily unavailable, though the 3-second delay mitigates excessive requests.
If you want to mark the check as complete even when errors occur (to avoid retry storms), consider moving the flag outside the try block:
Alternative: Always mark as checked (even on errors)
} catch (err) { if (err instanceof Error) { console.error(`Error checking for updates: ${err.message}`) } else { console.error('Unknown error checking for updates:', err) } + } finally { + // Mark as checked for this session (even if error occurred) + sessionStorage.setItem(VERSION_CHECK_KEY, 'true') } - - // Mark as checked for this session (regardless of whether update exists) - sessionStorage.setItem(VERSION_CHECK_KEY, 'true') }However, the current behavior (retry on failure) may be intentional and appropriate for this use case. Please confirm.
67-72: LGTM! Proper timeout and cleanup implementation.The 3-second delay prevents blocking the initial page load, and the cleanup function correctly clears the timeout if the component unmounts before the check executes. This follows React best practices for async operations in
useEffect.
Comment on lines
193
to
198
| ) : items.length === 0 && !isLoading ? ( | ||
| <div className="flex h-48 items-center justify-center"> | ||
| <span className="text-foreground text-muted-foreground"> | ||
| No recent requests | ||
| </span> | ||
| </div> |
Contributor
There was a problem hiding this comment.
Fix conflicting CSS classes.
Line 195 applies both text-foreground and text-muted-foreground, which are conflicting text color utilities. Only text-muted-foreground should be used for the "No recent requests" placeholder.
Proposed fix
- <span className="text-foreground text-muted-foreground">
+ <span className="text-muted-foreground">
No recent requests
</span>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| ) : items.length === 0 && !isLoading ? ( | |
| <div className="flex h-48 items-center justify-center"> | |
| <span className="text-foreground text-muted-foreground"> | |
| No recent requests | |
| </span> | |
| </div> | |
| ) : items.length === 0 && !isLoading ? ( | |
| <div className="flex h-48 items-center justify-center"> | |
| <span className="text-muted-foreground"> | |
| No recent requests | |
| </span> | |
| </div> |
🤖 Prompt for AI Agents
In @src/client/features/dashboard/components/recent-requests.tsx around lines
193 - 198, In the RecentRequests JSX
(src/client/features/dashboard/components/recent-requests.tsx) remove the
conflicting text color utility by deleting "text-foreground" from the
placeholder span and keep only "text-muted-foreground" on the <span> that
renders "No recent requests" so the muted color is applied consistently.
- Add invalid date validation in formatTimeAgo to return 'Unknown' - Use consistent version prefix formatting in update toast message
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @src/client/features/dashboard/components/recent-requests.tsx:
- Around line 154-159: The Button is nested inside a Link causing invalid nested
interactive elements; in recent-requests.tsx replace the <Link> wrapping the
<Button> with a single interactive element: import and call useNavigate from
react-router-dom inside the RecentRequests component and make the Button handle
onClick={() => navigate('/approvals')}, or alternatively remove the Button and
restyle the <Link> to look like a button using the Button's classes; ensure to
remove the surrounding <Link> and adjust imports (useNavigate) and the Button's
props accordingly.
🧹 Nitpick comments (3)
src/client/features/dashboard/components/charts/instance-content-breakdown-chart.tsx (1)
16-29: Consider memoizing CSS color extraction for better performance.The
cssColorsobject callsgetComputedStylesix times on every render. Since these CSS custom properties are static, wrapping this inuseMemowith an empty dependency array would avoid repeated DOM queries.⚡ Proposed optimization
+ const cssColors = useMemo(() => ({ - const cssColors = { grabbed: getComputedStyle(document.documentElement) .getPropertyValue('--chart-1') .trim() || '196 39% 33%', notified: getComputedStyle(document.documentElement) .getPropertyValue('--chart-3') .trim() || '29 85% 87%', requested: getComputedStyle(document.documentElement) .getPropertyValue('--chart-5') .trim() || '1 54% 50%', - } + }), [])src/client/features/dashboard/components/recent-requests.tsx (2)
39-46: Consider extracting shared responsive logic into a custom hook.The comment on line 39 indicates this media query logic is copied from WatchlistCarousel, and the carouselItemClass calculation (lines 85-116) implements similar responsive behavior. This code duplication creates maintenance overhead and risks divergence between components.
♻️ Suggested refactor to reduce duplication
Consider creating a shared hook that encapsulates the media query definitions and carousel item class calculation:
// src/client/features/dashboard/hooks/useCarouselResponsive.ts export function useCarouselResponsive(options: { fullscreenEnabled: boolean itemsMultiplier?: number // 2 for full-width, 1 for half-width }) { const isXXL = useMediaQuery('(min-width: 2450px)') const isXL = useMediaQuery('(min-width: 1900px)') const isLG = useMediaQuery('(min-width: 1600px)') const isMD2 = useMediaQuery('(min-width: 1200px)') const isMD = useMediaQuery('(min-width: 768px)') const isStacked = useMediaQuery('(max-width: 1279px)') const isSmallStacked = useMediaQuery('(max-width: 1100px)') const carouselItemClass = useMemo(() => { // Centralized responsive logic here // ... }, [/* dependencies */]) return { carouselItemClass } }Then use it in both RecentRequests and WatchlistCarousel:
- const isXXL = useMediaQuery('(min-width: 2450px)') - // ... other media queries - const carouselItemClass = useMemo(() => { - // ... complex logic - }, [...]) + const { carouselItemClass } = useCarouselResponsive({ + fullscreenEnabled, + itemsMultiplier: 2, // full-width component + })Also applies to: 85-116
73-81: Consider extracting magic numbers to named constants.The values
500(line 76) and10(line 201) are hard-coded. Extracting them to named constants would improve maintainability and make the intent clearer.♻️ Suggested refactor
+const MIN_LOADING_DURATION_MS = 500 +const SKELETON_ITEM_COUNT = 10 + export function RecentRequests() { // ... existing code ... useEffect(() => { if (isLoading) { setMinLoadingComplete(false) const timer = setTimeout(() => { setMinLoadingComplete(true) - }, 500) + }, MIN_LOADING_DURATION_MS) return () => clearTimeout(timer) } }, [isLoading]) // ... in JSX ... {isLoading && (!minLoadingComplete || items.length === 0) - ? Array.from({ length: 10 }, (_, i) => `skeleton-${i}`).map( + ? Array.from({ length: SKELETON_ITEM_COUNT }, (_, i) => `skeleton-${i}`).map( (skeletonId) => (Also applies to: 200-212
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
src/client/features/approvals/components/approval-radarr-routing-card.tsxsrc/client/features/approvals/components/approval-sonarr-routing-card.tsxsrc/client/features/content-router/components/accordion-route-card.tsxsrc/client/features/dashboard/components/charts/instance-content-breakdown-chart.tsxsrc/client/features/dashboard/components/charts/notification-charts.tsxsrc/client/features/dashboard/components/charts/top-genres-chart.tsxsrc/client/features/dashboard/components/recent-request-card.tsxsrc/client/features/dashboard/components/recent-requests.tsxsrc/client/features/dashboard/components/watchlist-carousel.tsxsrc/client/features/plex/pages/users.tsxsrc/client/features/radarr/components/instance/radarr-instance-card.tsxsrc/client/features/sonarr/components/instance/sonarr-instance-card.tsxsrc/client/hooks/useVersionCheck.tsx
✅ Files skipped from review due to trivial changes (5)
- src/client/features/dashboard/components/charts/top-genres-chart.tsx
- src/client/features/approvals/components/approval-radarr-routing-card.tsx
- src/client/features/dashboard/components/charts/notification-charts.tsx
- src/client/features/radarr/components/instance/radarr-instance-card.tsx
- src/client/features/content-router/components/accordion-route-card.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- src/client/hooks/useVersionCheck.tsx
- src/client/features/dashboard/components/watchlist-carousel.tsx
- src/client/features/dashboard/components/recent-request-card.tsx
🧰 Additional context used
🧠 Learnings (6)
📚 Learning: 2025-12-02T09:00:20.744Z
Learnt from: jamcalli
Repo: jamcalli/Pulsarr PR: 784
File: src/services/plex-watchlist/sync/item-categorizer.ts:29-46
Timestamp: 2025-12-02T09:00:20.744Z
Learning: In src/services/plex-watchlist/sync/item-categorizer.ts, the createWatchlistItem function intentionally uses `key: item.id` (not `templateItem.key`) when creating linked watchlist items for different users. This ensures consistent storage of the Plex ratingKey regardless of how the lookup was performed.
Applied to files:
src/client/features/plex/pages/users.tsx
📚 Learning: 2025-10-25T03:59:24.890Z
Learnt from: jamcalli
Repo: jamcalli/Pulsarr PR: 672
File: src/utils/plex/friends-api.ts:78-85
Timestamp: 2025-10-25T03:59:24.890Z
Learning: In src/utils/plex/friends-api.ts, the Friend object constructed from GraphQL only includes watchlistId and username. The userId field is intentionally populated later from the database, so the incomplete object construction is expected behavior.
Applied to files:
src/client/features/plex/pages/users.tsx
📚 Learning: 2025-08-03T17:04:36.623Z
Learnt from: jamcalli
Repo: jamcalli/Pulsarr PR: 431
File: src/services/plex-label-sync.service.ts:0-0
Timestamp: 2025-08-03T17:04:36.623Z
Learning: In the Plex label sync service, the user updated the `isAppUserLabel` method to use a dedicated `this.config.labelPrefix` property instead of extracting the prefix from `this.config.labelFormat` using substring operations. This matches the pattern used in the user tagging service and eliminates format validation concerns.
Applied to files:
src/client/features/plex/pages/users.tsx
📚 Learning: 2025-06-29T19:45:44.866Z
Learnt from: jamcalli
Repo: jamcalli/Pulsarr PR: 0
File: :0-0
Timestamp: 2025-06-29T19:45:44.866Z
Learning: When a PR involves Tailwind v3 to v4 auto-migration, most CSS class changes (like text-text → text-foreground, bg-bw → bg-secondary-background) are automated migration artifacts and don't require detailed review comments. Focus should be on architectural and functional changes rather than styling updates.
Applied to files:
src/client/features/sonarr/components/instance/sonarr-instance-card.tsxsrc/client/features/approvals/components/approval-sonarr-routing-card.tsxsrc/client/features/dashboard/components/recent-requests.tsx
📚 Learning: 2025-06-09T23:03:29.198Z
Learnt from: jamcalli
Repo: jamcalli/Pulsarr PR: 274
File: src/client/features/utilities/components/delete-sync/delete-sync-form.tsx:124-127
Timestamp: 2025-06-09T23:03:29.198Z
Learning: In the utilities dashboard AccordionTrigger components (delete-sync-form.tsx and similar), black text (`text-black`) is intentionally used instead of theme-aware classes for accessibility reasons. The white text did not provide sufficient contrast against the background, so black text is used across all themes to ensure readability.
Applied to files:
src/client/features/dashboard/components/recent-requests.tsx
📚 Learning: 2025-06-05T08:05:33.694Z
Learnt from: jamcalli
Repo: jamcalli/Pulsarr PR: 254
File: src/client/features/not-found/index.tsx:16-22
Timestamp: 2025-06-05T08:05:33.694Z
Learning: In the NotFoundPage component (src/client/features/not-found/index.tsx), the user prefers to keep hard-coded black text colors ("text-black" class) for better visual appearance, rather than relying on theme-based text colors.
Applied to files:
src/client/features/dashboard/components/recent-requests.tsx
🧬 Code graph analysis (1)
src/client/features/dashboard/components/recent-requests.tsx (2)
src/client/features/dashboard/hooks/useRecentRequests.ts (4)
useRecentRequests(53-97)STATUS_FILTER_OPTIONS(14-19)LIMIT_PRESETS(21-21)getLimitLabel(24-26)src/client/features/dashboard/components/recent-request-card.tsx (1)
RecentRequestCard(79-247)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (5)
src/client/features/sonarr/components/instance/sonarr-instance-card.tsx (1)
417-417: Consistent styling update for improved label visibility.The removal of
text-muted-foregroundfrom all five switch labels makes the text more visible and aligns with the broader theming update described in the PR summary.Also applies to: 458-458, 498-498, 538-538, 821-821
src/client/features/plex/pages/users.tsx (1)
177-177: LGTM! CSS conflict resolved.Removing
text-muted-foregroundwhentext-foregroundis already present eliminates the conflicting color declarations and clarifies the intended styling.src/client/features/approvals/components/approval-sonarr-routing-card.tsx (1)
313-313: LGTM! Conflicting text color class resolved.The removal of
text-muted-foregroundcorrectly resolves the CSS class conflict where two different text color utilities were applied to the same element. This aligns with the broader UI consistency improvements across the codebase.Based on learnings, styling-only changes from design token migrations don't require detailed review.
src/client/features/dashboard/components/charts/instance-content-breakdown-chart.tsx (1)
92-92: LGTM! Text color update aligns with design consistency.The removal of
text-muted-foregroundmakes the loading/empty state message more prominent and aligns with the broader UI design pass described in the PR objectives.src/client/features/dashboard/components/recent-requests.tsx (1)
29-71: Well-structured component with good practices.The component demonstrates several strong patterns:
- Proper hook usage and memoization to optimize performance
- Clean error, empty, and loading state handling (lines 189-196)
- Accessible navigation controls with sr-only labels (lines 174, 184)
- Unique and stable keys for list items (line 215)
- Smooth UX with minimum loading duration to prevent flashing content (lines 73-81)
- Proper cleanup of event listeners (lines 67-70)
Also applies to: 118-134, 136-230
…t loading state - Create useCarouselResponsive hook for shared responsive breakpoint logic - Remove minLoadingComplete state (already handled by useAppQuery's MIN_LOADING_DELAY) - Memoize cssColors in chart components to avoid repeated getComputedStyle calls - Simplify carousel conditional rendering
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @src/client/features/dashboard/components/recent-requests.tsx:
- Around line 90-103: The Select onValueChange handlers currently cast incoming
values unsafely (value as typeof status and Number(value) as typeof limit);
instead validate the incoming value against the known option sets before
updating state: for the status Select, check the received value exists in
filterOptions (e.g., compare to option.value or option.key) and only call
setStatus when it matches a known preset; for the limit Select, parse the value
to an integer (Number/parseInt), ensure it is a finite number and that it
matches one of the numeric entries in limitOptions (or falls back to a default),
and only then call setLimit; update the handlers around setStatus and setLimit
to perform these checks and early-return on invalid input to avoid setting
invalid status or NaN limit.
🧹 Nitpick comments (1)
src/client/features/dashboard/hooks/useCarouselResponsive.ts (1)
23-81: Likely double-padding:p-1is in the shared class and call sites addp-1wrappers.
Bothwatchlist-carousel.tsxandrecent-requests.tsxwrap contents in<div className="p-1">, whilecarouselItemClassalso includesp-1. This will compound spacing and is hard to reason about across consumers.Proposed fix: keep the hook responsible for sizing/gutters only (drop
p-1) and de-duplicate strings- const carouselItemClass = useMemo(() => { + const carouselItemClass = useMemo(() => { + const base = 'pl-2 md:pl-4' + const cls = (basis: string) => `${base} ${basis}` // When stacked (< 1280px): both components are full-width, same logic if (isStacked) { if (fullscreenEnabled) { - if (isMD2) return 'pl-2 md:pl-4 basis-1/3 p-1' // 3 items (1200-1279px) - if (isMD) return 'pl-2 md:pl-4 basis-1/2 p-1' // 2 items (768-1199px) - return 'pl-2 md:pl-4 basis-1/2 p-1' // 2 items (<768px) + if (isMD2) return cls('basis-1/3') // 3 items (1200-1279px) + return cls('basis-1/2') // 2 items (<1200px) } // Windowed stacked - if (isSmallStacked) return 'pl-2 md:pl-4 basis-1/2 p-1' // 2 items (≤1100px) - return 'pl-2 md:pl-4 basis-1/3 p-1' // 3 items (1101-1279px) + if (isSmallStacked) return cls('basis-1/2') // 2 items (≤1100px) + return cls('basis-1/3') // 3 items (1101-1279px) } // Side-by-side (>= 1280px): behavior differs based on fullWidth if (fullWidth) { // Full-width component: double the items if (fullscreenEnabled) { - if (isXXL) return 'pl-2 md:pl-4 basis-[10%] p-1' // 10 items - if (isXL) return 'pl-2 md:pl-4 basis-[12.5%] p-1' // 8 items - if (isLG) return 'pl-2 md:pl-4 basis-1/6 p-1' // 6 items - return 'pl-2 md:pl-4 basis-1/4 p-1' // 4 items + if (isXXL) return cls('basis-[10%]') // 10 items + if (isXL) return cls('basis-[12.5%]') // 8 items + if (isLG) return cls('basis-1/6') // 6 items + return cls('basis-1/4') // 4 items } // Windowed full-width - return 'pl-2 md:pl-4 basis-1/4 p-1' // 4 items + return cls('basis-1/4') // 4 items } // Half-width component (default): standard item count if (fullscreenEnabled) { - if (isXXL) return 'pl-2 md:pl-4 basis-1/5 p-1' // 5 items - if (isXL) return 'pl-2 md:pl-4 basis-1/4 p-1' // 4 items - if (isLG) return 'pl-2 md:pl-4 basis-1/3 p-1' // 3 items - if (isMD2) return 'pl-2 md:pl-4 basis-1/2 p-1' // 2 items - if (isMD) return 'pl-2 md:pl-4 basis-1/2 p-1' // 2 items - return 'pl-2 md:pl-4 basis-1/2 p-1' // 2 items + if (isXXL) return cls('basis-1/5') // 5 items + if (isXL) return cls('basis-1/4') // 4 items + if (isLG) return cls('basis-1/3') // 3 items + return cls('basis-1/2') // 2 items } // Windowed half-width - return 'pl-2 md:pl-4 basis-1/2 p-1' // 2 items + return cls('basis-1/2') // 2 items }, [
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/client/features/dashboard/components/charts/content-distribution-chart.tsxsrc/client/features/dashboard/components/charts/instance-content-breakdown-chart.tsxsrc/client/features/dashboard/components/charts/notification-charts.tsxsrc/client/features/dashboard/components/recent-requests.tsxsrc/client/features/dashboard/components/watchlist-carousel.tsxsrc/client/features/dashboard/hooks/useCarouselResponsive.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/client/features/dashboard/components/charts/notification-charts.tsx
- src/client/features/dashboard/components/charts/instance-content-breakdown-chart.tsx
🧰 Additional context used
🧠 Learnings (6)
📚 Learning: 2025-12-29T18:26:34.049Z
Learnt from: jamcalli
Repo: jamcalli/Pulsarr PR: 842
File: src/services/database/methods/analytics.ts:197-200
Timestamp: 2025-12-29T18:26:34.049Z
Learning: In SQLite usage within TypeScript code, ensure JSON_GROUP_ARRAY(DISTINCT col) is used for distinct aggregation of a single column. Do not use GROUP_CONCAT(DISTINCT col, delimiter) because the 2-argument form with DISTINCT and a delimiter is invalid. When reviewing analytics-related SQL strings in src/services/database/methods/analytics.ts, prefer JSON_GROUP_ARRAY(DISTINCT <column>) for distinct arrays and avoid the invalid GROUP_CONCAT(DISTINCT <column>, <delimiter>) pattern.
Applied to files:
src/client/features/dashboard/hooks/useCarouselResponsive.ts
📚 Learning: 2025-06-09T23:03:29.198Z
Learnt from: jamcalli
Repo: jamcalli/Pulsarr PR: 274
File: src/client/features/utilities/components/delete-sync/delete-sync-form.tsx:124-127
Timestamp: 2025-06-09T23:03:29.198Z
Learning: In the utilities dashboard AccordionTrigger components (delete-sync-form.tsx and similar), black text (`text-black`) is intentionally used instead of theme-aware classes for accessibility reasons. The white text did not provide sufficient contrast against the background, so black text is used across all themes to ensure readability.
Applied to files:
src/client/features/dashboard/components/recent-requests.tsx
📚 Learning: 2025-06-05T08:05:33.694Z
Learnt from: jamcalli
Repo: jamcalli/Pulsarr PR: 254
File: src/client/features/not-found/index.tsx:16-22
Timestamp: 2025-06-05T08:05:33.694Z
Learning: In the NotFoundPage component (src/client/features/not-found/index.tsx), the user prefers to keep hard-coded black text colors ("text-black" class) for better visual appearance, rather than relying on theme-based text colors.
Applied to files:
src/client/features/dashboard/components/recent-requests.tsx
📚 Learning: 2025-06-29T19:45:44.866Z
Learnt from: jamcalli
Repo: jamcalli/Pulsarr PR: 0
File: :0-0
Timestamp: 2025-06-29T19:45:44.866Z
Learning: When a PR involves Tailwind v3 to v4 auto-migration, most CSS class changes (like text-text → text-foreground, bg-bw → bg-secondary-background) are automated migration artifacts and don't require detailed review comments. Focus should be on architectural and functional changes rather than styling updates.
Applied to files:
src/client/features/dashboard/components/recent-requests.tsx
📚 Learning: 2025-12-22T23:33:36.019Z
Learnt from: jamcalli
Repo: jamcalli/Pulsarr PR: 833
File: src/client/features/notifications/components/notifications-skeleton.tsx:211-215
Timestamp: 2025-12-22T23:33:36.019Z
Learning: In the Pulsarr codebase, the pattern "Click here <ExternalLink /> for [context]." is used consistently across notification components for external documentation links. The maintainer prefers to keep this pattern for consistency rather than changing individual instances for accessibility concerns.
Applied to files:
src/client/features/dashboard/components/recent-requests.tsx
📚 Learning: 2025-05-29T07:37:33.428Z
Learnt from: jamcalli
Repo: jamcalli/Pulsarr PR: 226
File: src/client/components/ui/clear-settings-alert.tsx:40-48
Timestamp: 2025-05-29T07:37:33.428Z
Learning: The codebase consistently uses a pattern where async functions in onClick handlers (like onConfirm) are called without awaiting, and modals close immediately without waiting for the operation to complete. This is an intentional, consistent pattern across the codebase and should not be flagged as an issue in individual PRs unless the PR specifically aims to refactor this pattern.
Applied to files:
src/client/features/dashboard/components/recent-requests.tsx
🧬 Code graph analysis (1)
src/client/features/dashboard/components/watchlist-carousel.tsx (1)
src/client/features/dashboard/hooks/useCarouselResponsive.ts (1)
useCarouselResponsive(19-84)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (2)
src/client/features/dashboard/components/watchlist-carousel.tsx (2)
65-74: Good:reInitlistener is detached on unmount.
147-156: Use guids or a fallback key to avoid collisions from duplicate titles/counts.Multiple items can share the same title and count (e.g., different versions of "The Ring" both with 5 watchlists). The
guidsarray is the stable unique identifier available onContentStatand should be preferred:key={guids.length > 0 ? guids.join('|') :${title}-${count}}or similar fallback.
Comment on lines
90
to
103
| <Select | ||
| value={status} | ||
| onValueChange={(value) => setStatus(value as typeof status)} | ||
| options={filterOptions} | ||
| disabled={isLoading} | ||
| className="w-40" | ||
| /> | ||
| <Select | ||
| value={limit.toString()} | ||
| onValueChange={(value) => setLimit(Number(value) as typeof limit)} | ||
| options={limitOptions} | ||
| disabled={isLoading} | ||
| className="w-27.5" | ||
| /> |
Contributor
There was a problem hiding this comment.
Avoid unsafe casts in Select handlers (can set invalid status / NaN limit).
Even if today’s options are controlled, this is fragile.
Proposed fix: validate against known options/presets
<Select
value={status}
- onValueChange={(value) => setStatus(value as typeof status)}
+ onValueChange={(value) => {
+ const next = STATUS_FILTER_OPTIONS.find((o) => o.value === value)
+ if (next) setStatus(next.value)
+ }}
options={filterOptions}
disabled={isLoading}
className="w-40"
/>
<Select
value={limit.toString()}
- onValueChange={(value) => setLimit(Number(value) as typeof limit)}
+ onValueChange={(value) => {
+ const next = LIMIT_PRESETS.find((p) => p.toString() === value)
+ if (next !== undefined) setLimit(next)
+ }}
options={limitOptions}
disabled={isLoading}
className="w-27.5"
/>🤖 Prompt for AI Agents
In @src/client/features/dashboard/components/recent-requests.tsx around lines 90
- 103, The Select onValueChange handlers currently cast incoming values unsafely
(value as typeof status and Number(value) as typeof limit); instead validate the
incoming value against the known option sets before updating state: for the
status Select, check the received value exists in filterOptions (e.g., compare
to option.value or option.key) and only call setStatus when it matches a known
preset; for the limit Select, parse the value to an integer (Number/parseInt),
ensure it is a finite number and that it matches one of the numeric entries in
limitOptions (or falls back to a default), and only then call setLimit; update
the handlers around setStatus and setLimit to perform these checks and
early-return on invalid input to avoid setting invalid status or NaN limit.
- Use wider types in useRecentRequests to eliminate type casts in Select handlers - Fix double p-1 padding in carousel items (hook handles sizing only) - DRY refactor useCarouselResponsive with cls helper function - Remove unused isMD breakpoint
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/client/features/dashboard/components/recent-requests.tsx (1)
68-84: Consider removing unnecessaryuseMemocalls.Since
STATUS_FILTER_OPTIONSandLIMIT_PRESETSare stable const arrays, wrapping the.map()operations inuseMemoprovides no benefit. The map operations are inexpensive and the input arrays never change.♻️ Simplified version
- const filterOptions = useMemo( - () => - STATUS_FILTER_OPTIONS.map((opt) => ({ - label: opt.label, - value: opt.value, - })), - [], - ) + const filterOptions = STATUS_FILTER_OPTIONS.map((opt) => ({ + label: opt.label, + value: opt.value, + })) - const limitOptions = useMemo( - () => - LIMIT_PRESETS.map((preset) => ({ - label: getLimitLabel(preset), - value: preset.toString(), - })), - [], - ) + const limitOptions = LIMIT_PRESETS.map((preset) => ({ + label: getLimitLabel(preset), + value: preset.toString(), + }))
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/client/features/dashboard/components/recent-requests.tsxsrc/client/features/dashboard/hooks/useCarouselResponsive.tssrc/client/features/dashboard/hooks/useRecentRequests.ts
🧰 Additional context used
🧠 Learnings (7)
📚 Learning: 2025-12-29T18:26:34.049Z
Learnt from: jamcalli
Repo: jamcalli/Pulsarr PR: 842
File: src/services/database/methods/analytics.ts:197-200
Timestamp: 2025-12-29T18:26:34.049Z
Learning: In SQLite usage within TypeScript code, ensure JSON_GROUP_ARRAY(DISTINCT col) is used for distinct aggregation of a single column. Do not use GROUP_CONCAT(DISTINCT col, delimiter) because the 2-argument form with DISTINCT and a delimiter is invalid. When reviewing analytics-related SQL strings in src/services/database/methods/analytics.ts, prefer JSON_GROUP_ARRAY(DISTINCT <column>) for distinct arrays and avoid the invalid GROUP_CONCAT(DISTINCT <column>, <delimiter>) pattern.
Applied to files:
src/client/features/dashboard/hooks/useRecentRequests.tssrc/client/features/dashboard/hooks/useCarouselResponsive.ts
📚 Learning: 2025-06-09T23:03:29.198Z
Learnt from: jamcalli
Repo: jamcalli/Pulsarr PR: 274
File: src/client/features/utilities/components/delete-sync/delete-sync-form.tsx:124-127
Timestamp: 2025-06-09T23:03:29.198Z
Learning: In the utilities dashboard AccordionTrigger components (delete-sync-form.tsx and similar), black text (`text-black`) is intentionally used instead of theme-aware classes for accessibility reasons. The white text did not provide sufficient contrast against the background, so black text is used across all themes to ensure readability.
Applied to files:
src/client/features/dashboard/components/recent-requests.tsx
📚 Learning: 2025-06-05T08:05:33.694Z
Learnt from: jamcalli
Repo: jamcalli/Pulsarr PR: 254
File: src/client/features/not-found/index.tsx:16-22
Timestamp: 2025-06-05T08:05:33.694Z
Learning: In the NotFoundPage component (src/client/features/not-found/index.tsx), the user prefers to keep hard-coded black text colors ("text-black" class) for better visual appearance, rather than relying on theme-based text colors.
Applied to files:
src/client/features/dashboard/components/recent-requests.tsx
📚 Learning: 2025-06-29T19:45:44.866Z
Learnt from: jamcalli
Repo: jamcalli/Pulsarr PR: 0
File: :0-0
Timestamp: 2025-06-29T19:45:44.866Z
Learning: When a PR involves Tailwind v3 to v4 auto-migration, most CSS class changes (like text-text → text-foreground, bg-bw → bg-secondary-background) are automated migration artifacts and don't require detailed review comments. Focus should be on architectural and functional changes rather than styling updates.
Applied to files:
src/client/features/dashboard/components/recent-requests.tsx
📚 Learning: 2025-12-22T23:33:36.019Z
Learnt from: jamcalli
Repo: jamcalli/Pulsarr PR: 833
File: src/client/features/notifications/components/notifications-skeleton.tsx:211-215
Timestamp: 2025-12-22T23:33:36.019Z
Learning: In the Pulsarr codebase, the pattern "Click here <ExternalLink /> for [context]." is used consistently across notification components for external documentation links. The maintainer prefers to keep this pattern for consistency rather than changing individual instances for accessibility concerns.
Applied to files:
src/client/features/dashboard/components/recent-requests.tsx
📚 Learning: 2025-05-29T07:37:33.428Z
Learnt from: jamcalli
Repo: jamcalli/Pulsarr PR: 226
File: src/client/components/ui/clear-settings-alert.tsx:40-48
Timestamp: 2025-05-29T07:37:33.428Z
Learning: The codebase consistently uses a pattern where async functions in onClick handlers (like onConfirm) are called without awaiting, and modals close immediately without waiting for the operation to complete. This is an intentional, consistent pattern across the codebase and should not be flagged as an issue in individual PRs unless the PR specifically aims to refactor this pattern.
Applied to files:
src/client/features/dashboard/components/recent-requests.tsx
📚 Learning: 2025-04-24T21:05:18.033Z
Learnt from: jamcalli
Repo: jamcalli/Pulsarr PR: 110
File: src/services/database.service.ts:0-0
Timestamp: 2025-04-24T21:05:18.033Z
Learning: The getDetailedStatusTransitionMetrics method in database.service.ts should use JavaScript-based quartile calculations and outlier filtering instead of SQL PERCENTILE_CONT, as SQLite (used via better-sqlite3) doesn't support the PERCENTILE_CONT window function.
Applied to files:
src/client/features/dashboard/components/recent-requests.tsx
🧬 Code graph analysis (2)
src/client/features/dashboard/hooks/useRecentRequests.ts (3)
src/schemas/dashboard/recent-requests.schema.ts (4)
RecentRequestStatus(48-48)RecentRequestItem(50-50)RecentRequestsResponse(52-54)RecentRequestsResponseSchema(42-45)src/client/lib/useAppQuery.ts (1)
useAppQuery(28-72)src/client/lib/apiClient.ts (1)
apiClient(93-122)
src/client/features/dashboard/components/recent-requests.tsx (3)
src/client/features/dashboard/hooks/useRecentRequests.ts (4)
useRecentRequests(53-95)STATUS_FILTER_OPTIONS(14-19)LIMIT_PRESETS(21-21)getLimitLabel(24-26)src/client/features/dashboard/hooks/useCarouselResponsive.ts (1)
useCarouselResponsive(19-81)src/client/features/dashboard/components/recent-request-card.tsx (1)
RecentRequestCard(79-247)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (8)
src/client/features/dashboard/hooks/useCarouselResponsive.ts (3)
4-11: LGTM - Clear interface design.The interface clearly defines the hook's options with helpful documentation for the
fullWidthparameter.
19-31: LGTM - Well-organized media queries.The breakpoint setup is comprehensive and clearly documented, supporting both stacked and side-by-side layouts across a wide range of screen sizes.
33-80: LGTM - Excellent DRY refactor with theclshelper.The
clshelper function eliminates repetition and makes the responsive logic more maintainable. The multi-branch logic is well-structured and thoroughly documented with item counts at each breakpoint.src/client/features/dashboard/components/recent-requests.tsx (3)
29-41: LGTM - Proper hook composition.The component correctly integrates the responsive carousel logic with
fullWidth: true, which aligns with the full-width layout shown in the JSX.
43-66: LGTM - Proper carousel event handling.The carousel API integration follows best practices with proper event subscription, cleanup, and memoized callbacks.
86-183: LGTM - Solid component rendering with proper accessibility.The rendering logic is well-structured with:
- Appropriate error and empty state handling
- Proper accessibility labels for navigation controls (lines 126, 136)
- Unique keys for carousel items using
source-idcomposition- Clean separation between loading skeletons and actual data
The Select handlers correctly avoid type assertions by using broader string types, as mentioned in the commit message.
src/client/features/dashboard/hooks/useRecentRequests.ts (2)
12-44: LGTM - Well-organized constants and types.The exported constants, types, and helper function provide a clean API for consumers. The 30-second polling interval is appropriate for dashboard data that updates periodically.
53-95: LGTM - Excellent React Query integration.The hook properly leverages React Query features:
keepPreviousDataprevents jarring UI updates during refetches- Polling with
refetchIntervalkeeps data fresh- Conditional status parameter (lines 71-73) avoids unnecessary query params
- Schema validation ensures type safety at runtime
The error handling safely extracts the message when available.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Related Issues
Type of Change
Testing Performed
Screenshots
Checklist
Summary by CodeRabbit
New Features
Bug Fixes
Style
Tests
✏️ Tip: You can customize this high-level summary in your review settings.