Skip to content

.pr_agent_accepted_suggestions

Jump to bottom
qodo-merge-bot edited this page Jul 9, 2026 · 7 revisions
                     PR 832 (2026-07-08)                    
[reliability] AppCheck telemetry init race
AppCheck telemetry init race BoxLoreApplication calls setupAppCheck() before PostHogAndroid.setup(), but the App Check pre-warm callbacks immediately call AnalyticsHelper.trackAppCheckStatus(), which calls PostHog.capture() with no initialization guard. If the token task resolves quickly, the event can be lost (or behave inconsistently) because PostHog may not be ready yet.

Issue description

setupAppCheck() runs before PostHogAndroid.setup(), but setupAppCheck() may emit AnalyticsHelper.trackAppCheckStatus(...) from async callbacks. Since trackAppCheckStatus() calls PostHog.capture() directly (no init check), this creates a race where the App Check adoption/health event can be dropped or behave inconsistently.

Issue Context

  • BoxLoreApplication.onCreate() invokes setupAppCheck() before PostHog initialization.
  • setupAppCheck() pre-warms App Check and tracks status in the success/failure listeners.
  • AnalyticsHelper.trackAppCheckStatus() calls PostHog.capture() unguarded.

Fix Focus Areas

  • app/src/main/java/cx/aswin/boxcast/BoxLoreApplication.kt[20-35]
  • app/src/main/java/cx/aswin/boxcast/BoxLoreApplication.kt[84-112]
  • core/data/src/main/java/cx/aswin/boxcast/core/data/analytics/AnalyticsHelper.kt[118-132]

What to change

  • Prefer moving PostHogAndroid.setup(this, config) before setupAppCheck() so any telemetry emitted from App Check callbacks has PostHog ready.
  • Alternatively, delay/queue the trackAppCheckStatus call until after PostHog is initialized (e.g., by setting a flag after PostHogAndroid.setup and only capturing when true).
[maintainability] Duplicate EOE sentinel value
Duplicate EOE sentinel value SleepTimerPopup introduces END_OF_EPISODE_MINUTES = 999 while PlaybackRepository.setSleepTimer() separately uses the literal 999 to trigger end-of-episode behavior. Duplicating the sentinel across modules risks future drift that would break end-of-episode timers.

Issue description

The end-of-episode sleep timer sentinel (999) is duplicated: defined as END_OF_EPISODE_MINUTES in the UI layer and checked as a magic number in PlaybackRepository. If either side changes, end-of-episode selection will silently stop working.

Issue Context

  • UI emits minutes=999 for "End of episode".
  • PlaybackRepository enables EOE mode when durationMinutes == 999.

Fix Focus Areas

  • core/designsystem/src/main/java/cx/aswin/boxcast/core/designsystem/components/SleepTimerPopup.kt[45-62]
  • core/data/src/main/java/cx/aswin/boxcast/core/data/PlaybackRepository.kt[1766-1786]

What to change

  • Move the sentinel into a shared constant (e.g., core/model or core/data), or model the selection as a sealed type (FixedMinutes vs EndOfEpisode) so the UI cannot send a magic number.
[security] App Check token logged
App Check token logged NetworkModule injects the X-Firebase-AppCheck header, while HttpLoggingInterceptor runs at HEADERS level in debug builds without redacting that header. This will print App Check tokens to logcat in debug/test builds.

Issue description

Debug builds log request headers via OkHttp's HttpLoggingInterceptor.Level.HEADERS. Since the new interceptor adds X-Firebase-AppCheck, the token will be printed to logcat unless explicitly redacted.

Issue Context

  • appCheckInterceptor adds X-Firebase-AppCheck.
  • loggingInterceptor is configured for HEADERS in debug builds.

Fix Focus Areas

  • core/network/src/main/java/cx/aswin/boxcast/core/network/NetworkModule.kt[31-62]

What to change

  • Add loggingInterceptor.redactHeader("X-Firebase-AppCheck").
  • Consider also redacting X-App-Key if it is sensitive in logs.
  • Keep the interceptor ordering, but ensure redaction happens on the logging interceptor instance used by the client.
                     PR 830 (2026-07-08)                    
[performance] DB inspector eagerly composes lists
DB inspector eagerly composes lists DbInspectorSection renders history/podcasts via `forEach` inside a `Column`, which eagerly composes all rows when the tab is visible instead of virtualizing. With history emitting up to 300 items (and subscriptions unbounded), this can cause avoidable UI jank and slow tab switches.

Issue description

DbInspectorSection uses Column { history.forEach { ... } } / podcasts.forEach { ... }, which composes every row eagerly. This is a regression from the deleted dialog’s LazyColumn(items(...)) approach and can cause jank when opening the inspector or switching tabs.

Issue Context

  • ListeningHistoryDao.getAllHistory() returns up to 300 rows.
  • PodcastDao.getSubscribedPodcasts() returns all subscribed podcasts (no limit).

Fix Focus Areas

  • feature/home/src/main/java/cx/aswin/boxcast/feature/home/DebugScreen.kt[272-299]
  • core/data/src/main/java/cx/aswin/boxcast/core/data/database/ListeningHistoryDao.kt[35-44]
  • core/data/src/main/java/cx/aswin/boxcast/core/data/database/PodcastDao.kt[15-20]

Suggested fix

  • Replace the inner Column+forEach with a LazyColumn (or LazyColumn inside a fixed-height container) using items(...).
  • Optionally add basic keys:
    • items(history, key = { it.episodeId }) { ... }
    • items(podcasts, key = { it.podcastId }) { ... }
[observability] Sleep prompt analytics misclassified
Sleep prompt analytics misclassified SleepTimerPopup invokes onDismiss both for inactivity auto-hide and for the post-selection confirmation timeout, but MainActivity’s onDismiss always tracks decision="dismiss". As a result, a successful timer selection will also emit a "dismiss" decision event (and timeouts can’t be distinguished from manual dismiss).

Issue description

SleepTimerPopup calls onDismiss() for multiple semantics (manual dismiss, auto-hide timeout, and confirmation auto-dismiss). In MainActivity, onDismiss always logs trackLateNightSafeguardDecision("dismiss"), so analytics are wrong (timer-set flow records an extra dismiss) and you lose the ability to distinguish timeout vs user dismiss.

Issue Context

  • SleepTimerPopup triggers onDismiss() after autoHideMillis and after the confirmation delay.
  • MainActivity wires onDismiss to unconditional analytics decision dismiss.

Fix Focus Areas

  • core/designsystem/src/main/java/cx/aswin/boxcast/core/designsystem/components/SleepTimerPopup.kt[61-90]
  • app/src/main/java/cx/aswin/boxcast/MainActivity.kt[2341-2362]

Suggested fix

  • Change the API to carry a reason, e.g.:
    • enum class SleepTimerPopupDismissReason { Manual, Timeout, Confirmation }
    • onDismiss: (SleepTimerPopupDismissReason) -> Unit
  • In SleepTimerPopup:
    • Close icon -> Manual
    • auto-hide -> Timeout
    • confirmation delay -> Confirmation
  • In MainActivity:
    • Track dismiss only for Manual
    • Track ignore (or a new decision) for Timeout
    • Do not track dismiss for Confirmation (timer selection already tracks timer_set).
                     PR 816 (2026-07-06)                    
[correctness] Session counters never reset
Session counters never reset LearnViewModel.trackScreenExit() reports cumulative counts (dismiss/queue/play/etc.) because the counters are never cleared when a new Learn session begins. This makes later learn_screen_session events internally inconsistent (time_spent_seconds covers the latest interval while counts include prior intervals).

Issue description

LearnViewModel emits learn_screen_session with counters that are never reset, so session summaries become cumulative across visits/resumes.

Issue Context

The session timer is restarted on resume (onScreenResume()), but the action counters are not, so the reported metrics drift.

Fix Focus Areas

  • feature/explore/src/main/java/cx/aswin/boxcast/feature/explore/LearnViewModel.kt[38-66]

Suggested fix

  • Add a private resetTelemetrySession() that sets cardsDismissedCount/cardsQueuedCount/playsCount/podcastsClickedCount/infosClickedCount back to 0.
  • Call it when starting a new session (e.g., inside onScreenResume() when hasTrackedExit is true, alongside resetting sessionStartTime).
  • Optionally also reset after emitting in trackScreenExit() if you want each emitted event to represent an independent session.
                     PR 807 (2026-07-05)                    
[correctness] Accent color mismatch
Accent color mismatch LearnScreen derives the palette extraction image from state.data.questionsStack, but the UI renders the swipe stack from state.questionsStack; after shuffling or dismissing, the header tint can come from a different (or already dismissed) card than the one on top.

Issue description

LearnScreen extracts the accent color from state.data.questionsStack.firstOrNull() while the rendered card stack uses state.questionsStack. This desynchronizes the logo tint from the visible top card after shuffle/dismiss.

Issue Context

LearnUiState.Success carries questionsStack specifically for the active stack shown on screen.

Fix Focus Areas

  • feature/explore/src/main/java/cx/aswin/boxcast/feature/explore/LearnScreen.kt[153-235]

Suggested fix

  • Change the palette source to state.questionsStack.firstOrNull() (the same list passed into CuriosityCardStack).
  • Ensure the LaunchedEffect/remember keys are tied to that active top-card image so the tint updates when the top card changes.
                     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