Skip to content

.pr_agent_accepted_suggestions

Jump to bottom
qodo-merge-bot edited this page Jul 4, 2026 · 7 revisions
                     PR 803 (2026-07-04)                    
[reliability] Fixed text height risk
Fixed text height risk Multiple card components hard-code the text block to `height(58.dp)`, which can clip titles/subtitles under larger system font scales or future typography token changes.

Issue description

Several cards force their text area to a fixed 58.dp height. This makes the layout brittle: if font scale increases (accessibility) or typography sizes change, text can be clipped because the container cannot grow.

Issue Context

This pattern appears in multiple card components that display a 2-line title + 1-line subtitle.

Fix Focus Areas

  • Replace .height(58.dp) with a more resilient constraint:
    • use heightIn(min = 58.dp) and allow growth, or
    • use defaultMinSize(minHeight = 58.dp) / wrapContentHeight() depending on desired behavior.
  • If the goal is consistent card heights, consider measuring based on typography tokens or using minLines + consistent lineHeight without hard capping the container.

Fix Focus Areas (code references)

  • feature/home/src/main/java/cx/aswin/boxcast/feature/home/components/PodcastCard.kt[114-133]
  • feature/home/src/main/java/cx/aswin/boxcast/feature/home/components/CuratedEpisodeCard.kt[109-129]
  • feature/home/src/main/java/cx/aswin/boxcast/feature/home/components/ForYouSection.kt[408-428]
[performance] remember keyed by list
remember keyed by list `gridState` uses `remember(..., gridItems.list)` even though it only derives a simple loading/content string, adding avoidable list equality work during recompositions.

Issue description

gridState is memoized with gridItems.list as a key, which can trigger structural equality checks for the list during recompositions, even though the derived value only depends on isLoading, isFilterLoading, and whether the list is empty.

Issue Context

This code runs inside the Home feed composable where recompositions can be frequent.

Fix Focus Areas

  • Change the remember keys to only the needed scalars (e.g., gridItems.list.isEmpty() or gridItems.list.size).
  • Alternatively, use derivedStateOf without including the whole list as a key.

Fix Focus Areas (code references)

  • feature/home/src/main/java/cx/aswin/boxcast/feature/home/HomeScreen.kt[915-920]
                     PR 800 (2026-07-03)                    
[reliability] Stale last-seen keys accumulate
Stale last-seen keys accumulate HomeRoute and PodcastInfoViewModel write lastSeenEpisodeId for any viewed podcast with a latestEpisode, even when the podcast isn’t subscribed; those entries are only removed on unsubscribe, so they can persist indefinitely and grow the DataStore preferences set. This increases the work done on every preferences read because lastSeenEpisodesStream scans all prefs and filters by prefix.

Issue description

setLastSeenEpisodeId() is invoked for podcasts that may not be subscribed (e.g., Home recommendations and general PodcastInfo loads). Since removeLastSeenEpisodeId() is only called on unsubscribe, this leaves behind persistent per-podcast DataStore keys that can accumulate and slow preference reads/mapping.

Issue Context

The stored last-seen mapping is only meaningful for subscribed podcasts (both isEpisodeNew() helpers require subscribedAt > 0). Writing entries for non-subscribed podcasts provides no benefit but increases stored preference keys and the cost of lastSeenEpisodesStream.

Fix Focus Areas

  • feature/home/src/main/java/cx/aswin/boxcast/feature/home/HomeScreen.kt[207-226]
  • feature/home/src/main/java/cx/aswin/boxcast/feature/home/HomeViewModel.kt[231-235]
  • feature/info/src/main/java/cx/aswin/boxcast/feature/info/PodcastInfoViewModel.kt[285-291]
  • feature/info/src/main/java/cx/aswin/boxcast/feature/info/PodcastInfoViewModel.kt[326-330]

Suggested change

  • Only call setLastSeenEpisodeId(podcastId, episodeId) when the podcast is subscribed.
    • In HomeViewModel.markPodcastEpisodeAsSeen, check subscriptionRepository.isSubscribed(podcastId) before writing.
    • In PodcastInfoViewModel.loadPodcast, reuse the already-computed isSubscribed flag to guard both setLastSeenEpisodeId calls.
  • Optionally add a periodic cleanup routine (e.g., on app start) that removes last_seen_episode_id_* keys for podcastIds not in subscribedPodcastIds, if you want defense-in-depth.
[correctness] NEW badge ignores play status
NEW badge ignores play status Subscriptions Shows tab now computes the NEW badge solely from publish time + lastSeenId and no longer considers podcast.episodeStatus, so the badge can appear even when the latest episode is already completed/in-progress. This is misleading because LibraryViewModel explicitly enriches podcasts with episodeStatus from listening history.

Issue description

The Shows grid card NEW indicator no longer checks podcast.episodeStatus, so it can display NEW for podcasts whose latest episode is already completed or in progress.

Issue Context

LibraryViewModel enriches each subscribed podcast with episodeStatus derived from listening history. The Shows tab previously used this to only show the indicator for UNPLAYED, but the new isEpisodeNew()/hasRecentNew path ignores it.

Fix Focus Areas

  • feature/library/src/main/java/cx/aswin/boxcast/feature/library/SubscriptionsScreen.kt[1256-1281]
  • feature/library/src/main/java/cx/aswin/boxcast/feature/library/SubscriptionsScreen.kt[1331-1353]
  • feature/library/src/main/java/cx/aswin/boxcast/feature/library/LibraryViewModel.kt[103-138]

Suggested change

  • Gate the NEW badge with episode status, e.g.:
    • val shouldShowNew = (podcast.episodeStatus == EpisodeStatus.UNPLAYED) && isEpisodeNew(...)
    • or, if desired: podcast.episodeStatus != EpisodeStatus.COMPLETED.
  • Keep the lastSeenId suppression as-is so users can dismiss the badge by viewing the show.
                     PR 780 (2026-06-28)                    
[maintainability] Dead isDarkTheme parameter
Dead isDarkTheme parameter FullPlayerContent still requires an isDarkTheme parameter, but system-bar appearance now uses LocalEffectiveDarkTheme instead, leaving misleading API surface and no-op argument plumbing from call sites.

Issue description

FullPlayerContent keeps an isDarkTheme parameter, but the function no longer uses it after switching to LocalEffectiveDarkTheme.current. This makes call sites look meaningful while having no effect.

Issue Context

UnifiedPlayerSheet now passes effectiveDarkTheme into isDarkTheme, but FullPlayerContent ignores it.

Fix

Remove isDarkTheme from FullPlayerContent’s signature and update all call sites accordingly (or reintroduce using the parameter if you intentionally want the composable to be theme-agnostic).

Fix Focus Areas

  • feature/player/src/main/java/cx/aswin/boxcast/feature/player/FullPlayerContent.kt[79-94]
  • feature/player/src/main/java/cx/aswin/boxcast/feature/player/FullPlayerContent.kt[181-187]
  • feature/player/src/main/java/cx/aswin/boxcast/feature/player/UnifiedPlayerSheet.kt[543-548]

Clone this wiki locally