[TV] Show archived and sort options on podcast details

[sztomek]

Description

Brings the Apple TV podcast-details controls to Android TV: the All episodes header now hosts an episode sort order menu and a show / hide archived filter, mirroring PodcastDetailView on tvOS.

Persistence matches Apple TV exactly:

• Sort order persists to the podcast's synced episodesSortType (same as tvOS and mobile Android) — reflects across devices.
• Show / hide archived is stored locally in a TV-only preference, exactly like Apple TV's UserDefaults flag. It does not touch the podcast's synced showArchived, so toggling it on the TV doesn't change the phone. The preference is wiped on sign-out.

This PR also consolidates the TV local preferences: the previous per-feature TvPlaylistPreferences / TvPodcastPreferences are merged into one TvPreferences manager (single backing store, one clearAll()), used by both the playlist and podcast detail view models.

The controls are the same components the TV playlist details screen already uses. Those were private to that screen, so this PR extracts them into shared components (TvArchivedFilterButton, and a generic TvSortButton<T>) and points the playlist screen at them — no behaviour change there.

What changed

• TvEpisodeListControls.kt (new) — shared TvArchivedFilterButton + generic TvSortButton<T>, extracted from the playlist screen.
• EpisodesSortTypeLabel.kt (new, compose) — EpisodesSortType.displayLabel(), mirroring the existing PlaylistEpisodeSortType helper, reusing the existing episode_sort_* strings.
• TvPreferences.kt (new) — unified TV-local preference manager (replaces TvPlaylistPreferences + TvPodcastPreferences); holds the per-playlist and per-podcast archived flags; cleared on sign-out via TvSignOutManager.
• TvPodcastDetailsViewModel — episodes come from combine(podcastByUuidFlow(uuid).filterNotNull().flatMapLatest { … }, isShowingArchivedFlow), so a sort change (synced podcast) or an archived toggle (local flow) re-filters reactively. changeSortType persists to the podcast; toggleArchiveFilter flips the local preference. Loaded exposes archivedEpisodeCount and isShowingArchived.
• TvPodcastDetailsScreen — the “All episodes” title becomes a header row with the archived + sort controls (left-focus wired to the Follow button). When every episode is archived and the filter hides them, the controls stay reachable and an “all archived” message is shown instead of a dead-end empty state.

Notes / out of scope

• No analytics events were added — the existing TV view models (podcast + playlist details) don’t track anything, so adding events here would be inconsistent. Easy to add if wanted (iOS fires podcast_screen_toggle_archived / podcasts_screen_sort_order_changed).
• Sort options are shown in the same order the mobile app uses: Title A–Z, Title Z–A, Newest→Oldest, Oldest→Newest, Shortest→Longest, Longest→Shortest.

Testing Instructions

1. Open the Android TV app and go to a followed podcast → podcast details.
2. In the All episodes header, open the sort button (icon on the right): pick a different order → the list reorders and the choice is checked/persisted (reopen to confirm; it also reflects on the phone app).
3. Open the show / hide archived dropdown: toggle it → archived episodes appear / disappear; the choice persists across reopen.
4. From either control, press left on the D-pad → focus returns to the Follow button.
5. On a podcast where every episode is archived: with archived hidden you get an “all N episodes archived” message (not an empty dead-end) and can still open the archived dropdown to show them again.

Screenshots or Screencast

Checklist

• If this is a user-facing change, I have added an entry in CHANGELOG.md
• Ensure the linter passes (./gradlew spotlessApply to automatically apply formatting/linting)
• I have considered whether it makes sense to add tests for my changes
• All strings that need to be localized are in modules/services/localization/src/main/res/values/strings.xml (reused existing strings; none added)
• Any jetpack compose components I added or changed are covered by compose previews
• I have updated (or requested that someone edit) the Event Horizon schema to reflect any new or changed analytics.

I have tested any UI changes...

• with different themes
• with a landscape orientation
• with the device set to have a large display and font size
• for accessibility with TalkBack

[dangermattic]

	1 Warning
⚠️	This PR is larger than 500 lines of changes. Please consider splitting it into smaller PRs for easier and faster reviews.

Generated by 🚫 Danger

[sztomek]

@claude review this pls

[claude]

Claude finished @sztomek's task in 5m 21s —— View job

---

Review

• Gather context (read the full diff)
• Review the shared components + compose helper
• Review TvPreferences consolidation + sign-out
• Review TvPodcastDetailsViewModel / screen
• Review tests

Solid PR overall — the extraction into TvEpisodeListControls.kt is a clean lift-and-shift (I diffed the moved bodies against the deleted ones; behaviour is identical), EpisodesSortTypeLabel follows the existing PlaylistEpisodeSortTypeLabel convention exactly, and the new VM tests are a real improvement. A few things worth addressing:

---

1. Unused import will fail spotlessCheck 🔴

tv/…/playlists/details/TvPlaylistDetailsScreen.kt:58 still has import au.com.shiftyjelly.pocketcasts.images.R as IR, but the only IR.* usages in that file (ic_chevron_small_up, ic_sort) moved out with the extracted buttons — there are now zero references to IR in the file. ktlint's no-unused-imports is enabled (it's not in the ktLintConfigOverride disable list in build.gradle.kts), so ./gradlew spotlessCheck should reject this. Note that no lint/build job has run on this PR yet — CI only shows Validate Gradle Wrapper + Claude Code Review — so this hasn't been caught.

Fix this →

2. The episodes query is re-subscribed on every podcasts-table write

TvPodcastDetailsViewModel.kt:61-67:

val podcastEpisodesFlow = podcastManager.podcastByUuidFlow(podcastUuid)
    .filterNotNull()
    .flatMapLatest { updatedPodcast ->
        episodeManager.findEpisodesByPodcastOrderedFlow(updatedPodcast)
            .map { episodes -> updatedPodcast to episodes }
    }

podcastByUuidFlow is a Room @Query Flow, so it re-emits on any change to the podcasts table (Room invalidation is per-table, and Room doesn't dedupe identical rows). Every podcast refresh writes latest_episode_uuid / latest_episode_date / sync_status, playback-effect edits, folder moves, and unrelated podcasts' rows all trigger a re-emission — each one cancels the in-flight episodes query and starts a fresh one. stateIn dedupes the resulting Loaded values by data-class equality, so the UI won't thrash, but the DB churn is real.

Only episodesSortType actually affects which query is chosen, so gating on that fixes it and drops the Pair plumbing:

val podcastFlow = podcastManager.podcastByUuidFlow(podcastUuid).filterNotNull()
val episodesFlow = podcastFlow
    .distinctUntilChangedBy { it.episodesSortType }
    .flatMapLatest { episodeManager.findEpisodesByPodcastOrderedFlow(it) }
emitAll(
    combine(podcastFlow, episodesFlow, isShowingArchivedFlow) { podcast, episodes, isShowingArchived -> … },
)

(FWIW the filterNotNull() on a declared-non-null Flow<Podcast> looks redundant but is a legitimate guard — Room will happily emit null through a non-null Flow<T> when the row is missing. Worth a short comment so nobody "cleans it up".)

Fix this →

3. Blocking Room write dispatched on Default

TvPodcastDetailsViewModel.kt:86-89 launches updateEpisodesSortTypeBlocking (a blocking Room @Query write) on defaultDispatcher. The Default pool is sized to CPU count and is the wrong place to park a blocking DB call. TvSignOutManager already injects @IoDispatcher, so it's a one-line change to inject and use that here.

4. podcast_no_episodes_all_archived isn't a plural

TvPodcastDetailsScreen.kt:289 uses stringResource(LR.string.podcast_no_episodes_all_archived, episodeCount), which is "All %d episodes of this podcast have been archived" — with a single archived episode that renders "All 1 episodes of this podcast have been archived". The playlist screen's equivalent handles this with a plural (plurals/tv_playlist_all_archived). It matches mobile today, so this is a copy/i18n nit rather than a regression, but since the sibling TV screen already does it properly it's inconsistent within the same feature. (Would need a new plural, which conflicts with the "no strings added" checkbox — your call.)

5. AllEpisodesArchived is duplicated instead of shared

The new AllEpisodesArchived at TvPodcastDetailsScreen.kt:280-296 is byte-for-byte identical to the private one in TvPlaylistDetailsScreen.kt:176-192 except for the string. Given this PR's whole premise is extracting the shared controls, this one seems like it should move next to them with a text: String parameter.

6. Worth checking focus behaviour on a device for the empty↔non-empty transitions

Moving LaunchedEffect(Unit) { firstEpisodeFocusRequester.requestFocus() } into the episodes.isNotEmpty() branch (TvPodcastDetailsScreen.kt:247-250) means it re-fires every time the list goes empty → non-empty. That's a path the archived toggle newly makes user-reachable: on an all-archived podcast, choosing "show archived" will yank focus out of the dropdown you just used and onto the first episode. Probably acceptable, arguably even desirable.

The reverse direction is the one I'd actually verify: focus an episode, then open the archived dropdown and hide archived on an all-archived podcast. The focused episode is removed from composition and nothing in the AllEpisodesArchived branch claims focus, so there's a risk the D-pad goes dead until you press something that re-anchors. Testing step 5 only covers the dropdown→dropdown path.

7. Test gap: the sort re-query itself is never exercised

podcastByUuidFlow is stubbed as MutableStateFlow(podcast) that never changes, so the flatMapLatest — the core new plumbing that makes a sort change re-order the list — has no coverage. changeSortType is only verified as "calls the manager". A test that emits a second podcast with a different episodesSortType and asserts findEpisodesByPodcastOrderedFlow is invoked again with it (and the new list surfaces in Loaded) would pin the actual behaviour. Also missing: toggling archived back off re-filters.

8. The prefs-file rename silently drops existing playlist state

tv_playlists → tv_preferences means any existing install loses its per-playlist archived flag, and the orphaned tv_playlists.xml sticks around and is no longer wiped by clearAll() on sign-out. If nothing has shipped yet this is a non-issue — just flagging that the consolidation isn't data-preserving, since the keys themselves are unchanged and only the file name moved.

Nits

• TvPreferences lives in au.com.shiftyjelly.pocketcasts.preferences, the same package as :modules:services:preferences — TvSignOutManager now imports …preferences.Settings and …preferences.TvPreferences from two different modules. A split package across modules works fine on Android but makes imports ambiguous to read; …tv.preferences would be clearer.
• No @Preview for the newly-shared TvArchivedFilterButton / TvSortButton, though the checklist claims preview coverage. (The podcast screen preview was updated.)
• TvSortButton takes no leftFocusRequester while TvArchivedFilterButton does. Correct in both current call sites (the archived button is always to its left), just asymmetric for something now public and generic.
• The hardcoded PodcastSortOptions list is justified — EpisodesSortType.entries has DATE_ASC before DATE_DESC, so entries wouldn't give the mobile ordering.

Nothing here is a blocker except #1, which will break spotlessCheck.
· feat/tv-podcast-details-sort-archived

[claude]

Claude finished @sztomek's task in 4m 21s —— View job

---

Review (re-review after 9cbef22 Address PR comments)

• Read the full diff vs feat/podcasts-details-episodes
• Check which of the earlier review points the latest commit addresses
• Review shared components + compose helper
• Review TvPreferences + sign-out
• Review TvPodcastDetailsViewModel / screen
• Review tests

Previous round: what got fixed

#	Item	Status
1	Unused IR import in TvPlaylistDetailsScreen	✅ removed — I re-checked every import in all changed files, none are now dead
2	Episodes query re-subscribed on every podcasts write	✅ distinctUntilChangedBy { it.episodesSortType } before flatMapLatest, Pair plumbing gone
3	Blocking Room write on Default	✅ @IoDispatcher injected and used in changeSortType
7	No coverage for the sort re-query	✅ new a sort order change re-runs the episodes query test (see #E below for a tightening)
4, 5, 6, 8	plural string, AllEpisodesArchived duplication, focus transitions, prefs-file rename	Not addressed — carried forward

I also verified the two things the PR description asserts about persistence: PodcastDao.updateEpisodesSortTypeBlocking sets episodes_sort_order_modified + sync_status = 0, so sort really does sync; and findByPodcastOrderTitleAscFlow & friends don't filter on archived, so archivedEpisodeCount counts the full set correctly.

---

New findings

A. No scroll reset after a sort change — the list lands at an arbitrary offset 🟡

TvPodcastDetailsScreen.kt:255-259 uses a bare LazyColumn with no LazyListState. TvPlaylistDetailsScreen.kt:141-144 deliberately handles exactly this case:

val listState = rememberLazyListState()
LaunchedEffect(uiState.playlist.settings.sortType) {
    listState.scrollToItem(0)
}

Without it, and with key = { _, episode -> episode.uuid } on the items, LazyColumn re-anchors on the previously-first-visible episode's key after the reorder. Concretely: scroll 40 episodes down in Newest→Oldest, open the sort menu (focus is now on the sort button, so there's no focused item to re-anchor on either), pick Title A–Z — the viewport jumps to wherever that one episode landed alphabetically rather than the top of the newly sorted list. Same feature, sibling screen, opposite behaviour.

Fix this →

B. podcastFlow is collected twice → two Room subscriptions for the same row

TvPodcastDetailsViewModel.kt:63-67: podcastFlow is passed to combine and consumed by episodesFlow. It's a cold Room @Query flow, so that's two independent query executions plus two invalidation observers on the podcasts table for the same single row. Not a correctness problem and far cheaper than what #2 fixed, but a shareIn(viewModelScope, SharingStarted.WhileSubscribed(), replay = 1) (or restructuring so episodesFlow is derived inside the combine) collapses it to one.

C. combine briefly pairs the new podcast with the old episode order

Also TvPodcastDetailsViewModel.kt:68-77: on a sort change, podcastFlow emits the updated podcast immediately, while episodesFlow has to re-subscribe and re-run the query. combine therefore emits one intermediate Loaded where the sort menu's checkmark has already moved but episodes is still in the old order. It self-corrects within a frame or two — flagging it because it's the kind of thing that looks like a bug on a slow TV device, and because with A's scrollToItem(0) keyed on episodesSortType the scroll reset would fire against the old list.

D. Selecting the already-active sort option writes to the DB and dirties sync

TvEpisodeListControls.kt:124-127 calls onSelect(option) unconditionally, so re-picking the current sort runs updateEpisodesSortTypeBlocking → episodes_sort_order_modified = now, sync_status = 0, queuing a pointless sync for a no-op. A if (option != selected) guard in TvSortButton fixes it for both call sites (the playlist screen has the same behaviour today via playlistManager.updateSortType, so this is a pre-existing wart the extraction inherits rather than something this PR introduces).

E. The new sort test doesn't pin the sort type

TvPodcastDetailsViewModelTest.kt:157 asserts verify(episodeManager, times(2)).findEpisodesByPodcastOrderedFlow(any()). That confirms a re-subscription happened but not that it was driven by the sort type — the same assertion passes if the distinctUntilChangedBy key is wrong. verify(episodeManager).findEpisodesByPodcastOrderedFlow(argThat { episodesSortType == EPISODES_SORT_BY_TITLE_ASC }) would pin the actual contract. The complementary negative case is the more valuable one and is still missing: emit a podcast that differs in some other field (e.g. latestEpisodeUuid) and assert findEpisodesByPodcastOrderedFlow is not called again — that's the regression guard for #2, and nothing currently covers it.

Fix this →

---

Carried forward from the previous round

• podcast_no_episodes_all_archived isn't a plural (TvPodcastDetailsScreen.kt:289) — renders "All 1 episodes of this podcast have been archived". The sibling tv_playlist_all_archived is a proper plural with a dedicated one form. Still a copy/i18n nit rather than a regression (matches mobile), but inconsistent within the same feature.
• AllEpisodesArchived is duplicated — TvPodcastDetailsScreen.kt:280-296 is identical to TvPlaylistDetailsScreen.kt:182-198 apart from the string. Given the PR's premise is extracting the shared controls, it belongs next to them with a text: String param. Fix this →
• Focus on the empty ↔ non-empty transitions still wants a device check. LaunchedEffect(Unit) { firstEpisodeFocusRequester.requestFocus() } now lives inside the episodes.isNotEmpty() branch. Show-archived on an all-archived podcast yanks focus from the dropdown to the first episode (probably fine). The reverse is the one to verify: focus an episode, then hide archived on an all-archived podcast — the focused node leaves composition and nothing in the AllEpisodesArchived branch claims focus, so the D-pad may go dead. Testing step 5 only exercises dropdown → dropdown.
• The tv_playlists → tv_preferences rename isn't data-preserving — keys are unchanged, only the file moved, so existing installs lose their per-playlist archived flag and the orphaned tv_playlists.xml is no longer wiped by clearAll(). A non-issue if nothing has shipped.
• No @Preview for the newly-shared TvArchivedFilterButton / TvSortButton, though the checklist claims preview coverage.
• Package placement — TvPreferences sits in au.com.shiftyjelly.pocketcasts.preferences, the same package as :modules:services:preferences; TvSignOutManager now imports Settings and TvPreferences from two different modules under one package name. …tv.preferences reads better.

Notes

• EpisodesSortTypeLabel.displayLabel() is exhaustive over all six EpisodesSortType entries with no else, so a new sort type would be a compile error rather than a silent fallthrough. 👍
• The hardcoded PodcastSortOptions ordering remains justified (entries has DATE_ASC before DATE_DESC).
• No build/lint/test job has run on this PR — CI shows only Validate Gradle Wrapper ×2 and Claude Code Review. I verified imports and read the code but did not run ./gradlew spotlessCheck or the tv unit tests here, so nothing has actually compiled the branch.

Nothing here is a blocker. A is the one I'd fix before merge — it's a visible behavioural divergence from the sibling screen in the very interaction this PR adds.
· feat/tv-podcast-details-sort-archived

[claude]

Claude finished @sztomek's task in 2m 0s —— View job

---

Review (re-review after force-push 6261d5c)

• Read the full diff vs feat/podcasts-details-episodes
• Check what changed since the previous review
• Re-review view model / screen / shared components / tests

This push contains no code changes. The branch was rebased onto the new base tip (4eb61a2 Open podcast details from the Home featured and video tiles) — same 8 commit subjects, new SHAs, all re-timestamped, and the resulting tree is identical to what I reviewed at 9cbef22. git merge-base HEAD origin/feat/podcasts-details-episodes is now the base branch tip, so the rebase is clean, and the diffstat is unchanged (493/205 across 12 files).

I re-read TvPodcastDetailsViewModel, TvPodcastDetailsScreen, TvEpisodeListControls, TvPreferences, TvSignOutManager, TvPlaylistDetailsViewModel/Screen and both test files to confirm rather than assume. Everything from the previous round still applies verbatim — nothing regressed, nothing new fixed.

---

Still outstanding

The one I'd fix before merge:

A. No scroll reset after a sort change 🟡

TvPodcastDetailsScreen.kt:251-254 — bare LazyColumn, no LazyListState. The sibling screen handles exactly this at TvPlaylistDetailsScreen.kt:141-144:

val listState = rememberLazyListState()
LaunchedEffect(uiState.playlist.settings.sortType) {
    listState.scrollToItem(0)
}

With key = { _, episode -> episode.uuid } on the items, LazyColumn re-anchors on the previously-first-visible episode after a reorder, so scrolling 40 episodes down and then switching to Title A–Z lands the viewport wherever that one episode ended up alphabetically instead of at the top. Same feature, sibling screen, opposite behaviour. Fix this →

The rest, condensed:

	Item	Where
B	podcastFlow is collected twice (by combine and by episodesFlow) → two Room query executions + two invalidation observers for the same row. A shareIn(viewModelScope, WhileSubscribed(), replay = 1) collapses it.	TvPodcastDetailsViewModel.kt:63-68
C	On a sort change combine emits one intermediate Loaded pairing the new podcast with the old episode order (checkmark moves before the list does). Self-corrects in a frame; interacts with A's scrollToItem(0) if that's keyed on episodesSortType.	TvPodcastDetailsViewModel.kt:68-75
D	Re-picking the already-selected sort option still calls onSelect → updateEpisodesSortTypeBlocking sets sync_status = 0 for a no-op. An if (option != selected) guard in TvSortButton fixes both call sites. (Pre-existing on the playlist side; inherited by the extraction.)	TvEpisodeListControls.kt:124-127
E	verify(episodeManager, times(2)).findEpisodesByPodcastOrderedFlow(any()) confirms a re-subscription, not that the sort type drove it — passes even with a wrong distinctUntilChangedBy key. The negative case (podcast differing only in e.g. latestEpisodeUuid must not re-subscribe) is the actual regression guard and is still missing.	TvPodcastDetailsViewModelTest.kt:157
4	podcast_no_episodes_all_archived isn't a plural → "All 1 episodes … archived". TvPlaylistDetailsScreen.kt:191 uses a proper pluralStringResource.	TvPodcastDetailsScreen.kt:289
5	AllEpisodesArchived duplicated byte-for-byte (modulo the string) across both screens — belongs in TvEpisodeListControls.kt with a text: String param, given the PR's premise.	TvPodcastDetailsScreen.kt:280-296 / TvPlaylistDetailsScreen.kt:182-198
6	Focus on empty ↔ non-empty transitions still wants a device check: hide-archived on an all-archived podcast removes the focused episode from composition and nothing in the AllEpisodesArchived branch claims focus. Testing step 5 only covers dropdown → dropdown.	TvPodcastDetailsScreen.kt:247-250
8	tv_playlists → tv_preferences isn't data-preserving (keys unchanged, only the file moved), and the orphaned tv_playlists.xml is no longer wiped by clearAll(). Non-issue if nothing shipped.	TvPreferences.kt:13
—	No @Preview for the newly-shared TvArchivedFilterButton / TvSortButton; TvPreferences shares the …pocketcasts.preferences package name with :modules:services:preferences (…tv.preferences reads better).

CI

Still no build, lint, or test job on this PR — CI shows only Validate Gradle Wrapper ×2 and Claude Code Review. I verified imports and read the code but did not compile the branch or run the tv unit tests, so nothing has actually built it.
· feat/tv-podcast-details-sort-archived

[wpmobilebot]

Version 8.18 has now entered code-freeze, so the milestone of this PR has been updated to 8.19.

[claude]

Claude finished @sztomek's task in 3m 9s —— View job

---

Review (re-review after 01dfe9b)

• Read the full diff vs feat/podcasts-details-episodes
• Determine what changed since the previous review
• Re-check the outstanding items (A–E, 4–8)
• Look for new issues

This push contains no code changes. The base branch was itself rebased (Open podcast details from the Home featured and video tiles moved 4eb61a2 → 4622561) and this branch was rebased on top of it. Same 8 commit subjects, new SHAs, git merge-base HEAD origin/feat/podcasts-details-episodes is the base tip, and the diffstat is 497/209 across 12 files — byte-identical content to what I reviewed at 9cbef22 / 6261d5c.

I re-read every changed file rather than assuming, and re-verified two upstream facts the design depends on: EpisodeManagerImpl.findEpisodesByPodcastOrderedFlow dispatches purely on podcast.episodesSortType (so distinctUntilChangedBy { it.episodesSortType } is the correct and complete key), and none of the findByPodcastOrder*Flow DAO queries filter on archived (EpisodeDao.kt:200-235), so archivedEpisodeCount counts the full set correctly.

One thing I hadn't spotted in the earlier rounds, which the sibling-screen comparison makes clear:

---

F. The initial focus request isn't hardened the way the sibling screen's is 🟡 (new)

TvPodcastDetailsScreen.kt:247-250:

val firstEpisodeFocusRequester = remember { FocusRequester() }
LaunchedEffect(Unit) {
    firstEpisodeFocusRequester.requestFocus()
}

versus TvPlaylistDetailsScreen.kt:149-161, which does the same job with three deliberate guards:

LaunchedEffect(uiState.episodes.isNotEmpty()) {
    if (uiState.episodes.isNotEmpty() && !hasRequestedInitialFocus) {
        snapshotFlow { listState.layoutInfo.visibleItemsInfo.isNotEmpty() }.first { it }  // wait for layout
        runCatching { firstEpisodeFocusRequester.requestFocus() }                          // don't throw
        hasRequestedInitialFocus = true                                                    // one-shot
    }
}

The requester is attached to a LazyColumn item (TvEpisodeListItem.kt:54 → Modifier.focusRequester), which is only composed during the list's measure pass — not during the composition that launches the effect. FocusRequester.requestFocus() on an unattached requester throws IllegalStateException("FocusRequester is not initialized"), uncaught inside the LaunchedEffect coroutine. That snapshotFlow { … }.first { it } + runCatching pair on the playlist side reads exactly like the fix for that, and this PR doesn't carry it over.

This PR is also what makes the race newly reachable mid-session: because the effect now lives inside the episodes.isNotEmpty() branch (moved there by this PR), choosing "show archived" on an all-archived podcast enters a brand-new LazyColumn and re-fires requestFocus() at the worst possible moment — the same empty → non-empty transition the playlist screen guards against. The reverse direction is the other half of the old point 6: hiding archived removes the focused episode from composition and nothing in the AllEpisodesArchived branch claims focus.

Reusing the playlist screen's pattern (or, better, extracting it alongside the controls) covers both. Fix this →

Note this pairs naturally with A below — both want a hoisted rememberLazyListState(), so they're one edit.

---

Still outstanding from the previous rounds

A. No scroll reset after a sort change 🟡

TvPodcastDetailsScreen.kt:251-254 — bare LazyColumn, no LazyListState. TvPlaylistDetailsScreen.kt:152-154 handles exactly this:

LaunchedEffect(uiState.playlist.settings.sortType) {
    listState.scrollToItem(0)
}

With key = { _, episode -> episode.uuid }, LazyColumn re-anchors on the previously-first-visible episode after a reorder, so scrolling 40 episodes down and switching to Title A–Z lands the viewport wherever that one episode ended up alphabetically instead of at the top. Same feature, sibling screen, opposite behaviour. Fix this →

The rest, condensed:

	Item	Where
B	podcastFlow is collected twice (by combine and by episodesFlow) → two Room query executions + two invalidation observers for the same row. A shareIn(viewModelScope, WhileSubscribed(), replay = 1) collapses it.	TvPodcastDetailsViewModel.kt:63-68
C	On a sort change combine emits one intermediate Loaded pairing the new podcast with the old episode order (the checkmark moves before the list does). Self-corrects in a frame; interacts with A if the scrollToItem(0) is keyed on episodesSortType.	TvPodcastDetailsViewModel.kt:68-75
D	Re-picking the already-selected sort option still calls onSelect → updateEpisodesSortTypeBlocking sets sync_status = 0 for a no-op. An if (option != selected) guard in TvSortButton fixes both call sites. (Pre-existing on the playlist side; inherited by the extraction. Note TvArchivedFilterButton does guard this — TvEpisodeListControls.kt:69,79 — so the asymmetry is within one new file.)	TvEpisodeListControls.kt:124-127
E	verify(episodeManager, times(2)).findEpisodesByPodcastOrderedFlow(any()) confirms a re-subscription, not that the sort type drove it — it passes even with a wrong distinctUntilChangedBy key. The negative case (a podcast differing only in e.g. latestEpisodeUuid must not re-subscribe) is the actual regression guard and is still missing.	TvPodcastDetailsViewModelTest.kt:157
4	podcast_no_episodes_all_archived isn't a plural → "All 1 episodes … archived". TvPlaylistDetailsScreen.kt:209 uses a proper pluralStringResource.	TvPodcastDetailsScreen.kt:289
5	AllEpisodesArchived duplicated byte-for-byte (modulo the string) across both screens — belongs in TvEpisodeListControls.kt with a text: String param, given the PR's premise.	TvPodcastDetailsScreen.kt:280-296 / TvPlaylistDetailsScreen.kt:199-215
8	tv_playlists → tv_preferences isn't data-preserving (keys unchanged, only the file moved), and the orphaned tv_playlists.xml is no longer wiped by clearAll(). Non-issue if nothing shipped.	TvPreferences.kt:14
—	No @Preview for the newly-shared TvArchivedFilterButton / TvSortButton; TvPreferences shares the …pocketcasts.preferences package name with :modules:services:preferences, so TvSignOutManager imports Settings and TvPreferences from two modules under one package (…tv.preferences reads better).

CI

Still no build, lint, or test job on this PR — CI shows only Validate Gradle Wrapper ×2 plus this review job. I read the code and verified imports and the upstream DAO/manager behaviour, but did not compile the branch or run the tv unit tests, so nothing has actually built it.

Recommendation: F and A are one edit (both need a hoisted rememberLazyListState) and both are divergences from the sibling screen in the exact interaction this PR adds — I'd do that before merge. Everything else is optional polish.
· feat/tv-podcast-details-sort-archived

[claude]

Claude finished @sztomek's task in 4m 56s —— View job

---

Review (re-review after the latest push)

• Read the full diff vs feat/podcasts-details-episodes (now at 486c24c)
• Determine what changed since the previous review
• Re-check the outstanding items (A–F, 4–8)
• Look for new issues

This push has real changes this time (unlike the last two, which were rebases): the scroll reset, the hardened initial-focus effect, and the no-op sort guard all landed. Note the base branch also moved — TvPlaylistDetailsScreen/TvPodcastDetailsScreen upstream now already carry rememberLazyListState + snapshotFlow/runCatching, so part of what I flagged as F was fixed upstream rather than here.

Fixed

#	Item	Where
A	Scroll reset after a sort change	TvPodcastDetailsScreen.kt:220-222 — LaunchedEffect(podcast.episodesSortType) { listState.scrollToItem(0) }
F	Initial focus request hardened + made one-shot	TvPodcastDetailsScreen.kt:223-231 — hasRequestedInitialFocus, snapshotFlow { … index == 0 }.first { it }, runCatching with a Timber log
D	Re-picking the already-selected sort no longer writes to the DB	TvEpisodeListControls.kt:124-128 — if (option != selected); this also fixes the pre-existing wart on the playlist screen, since SortDropdownButton had no such guard
—	Test coverage: toggling archived back off, the stored-preference path, and archivedEpisodeCount	TvPodcastDetailsViewModelTest.kt

I also re-checked the A/C interaction I was worried about: on a sort change combine emits (new podcast, old episodes) first, so scrollToItem(0) runs against the old order and the reordered list arrives a frame later. As far as I can tell that's benign — LazyListState.scrollToItem goes through requestPositionAndForgetLastKnownKey, which drops the remembered first-visible key, so the subsequent data change can't re-anchor the viewport back onto the old first episode. Worth a quick device check regardless, since it's the one path where the two emissions are observable.

---

New this round

G. The scroll reset is unguarded, unlike the sibling screen's 🟢

TvPodcastDetailsScreen.kt:220-222:

LaunchedEffect(podcast.episodesSortType) {
    listState.scrollToItem(0)
}

vs TvPlaylistDetailsScreen.kt:152-157, which deliberately skips the first run:

var lastSortType by remember { mutableStateOf(sortType) }
LaunchedEffect(sortType) {
    if (sortType != lastSortType) { lastSortType = sortType; listState.scrollToItem(0) }
}

rememberLazyListState is rememberSaveable, so the unguarded version fires once on entry and discards any restored scroll offset (config change, process-death restore, re-entering the screen).

For this screen the net effect is arguably an improvement rather than a bug: the focus effect waits on visibleItemsInfo.any { it.index == 0 }, which would never become true if the list restored at index 40, so previously nothing got focused after a restore — forcing the list to the top makes that request reachable. But it does mean the podcast screen always returns to the top while the playlist screen restores position, which is the same "same feature, sibling screen, opposite behaviour" asymmetry in the other direction. Fine to leave as an intentional choice; just worth being a choice.

H. hasRequestedInitialFocus is initialised to true when the list is empty — the name hides the intent

TvPodcastDetailsScreen.kt:223:

var hasRequestedInitialFocus by remember { mutableStateOf(episodes.isEmpty()) }

This is what stops "show archived" on an all-archived podcast from yanking focus off the dropdown you just used (old point 6) — but a flag called hasRequestedInitialFocus starting at true before anything has been requested reads as a bug at a glance. It mirrors TvPlaylistDetailsScreen.kt:166, so consistency is fine; a one-line comment ("empty list → never claim focus, so toggling archived doesn't steal it from the controls") would stop someone "fixing" it later.

The consequence still wants a device check, and it's exactly testing step 5: entering an all-archived podcast leaves nothing focused — the Follow button isn't requested either, and EpisodeList composes with an empty list. Same as the sibling screen and the same as the pre-existing NoEpisodes path, so not a regression, but the first D-pad press relies on Compose's initial focus search rather than moving from a known anchor. The reverse direction (focus an episode, then hide archived) is unchanged from my earlier note: the focused node leaves composition and the AllEpisodesArchived branch claims nothing.

FYI (base branch, not this PR)

TvPlaylistDetailsScreen.kt:168-181 has two near-identical LaunchedEffect(uiState.episodes.isNotEmpty()) initial-focus blocks — both launch, both see !hasRequestedInitialFocus, both call requestFocus(). Harmless but clearly an accidental duplicate, and it's in a file this PR is already touching, so cheap to delete while you're in there. Fix this →

---

Still outstanding

	Item	Where
B	podcastFlow is collected twice (by combine and by episodesFlow) → two Room query executions + two invalidation observers for the same row. shareIn(viewModelScope, WhileSubscribed(), replay = 1) collapses it.	TvPodcastDetailsViewModel.kt:63-68
C	combine emits one intermediate Loaded pairing the new podcast with the old episode order (checkmark moves before the list does). Self-corrects in a frame; see the note above on why it doesn't defeat A.	TvPodcastDetailsViewModel.kt:68-75
E	verify(episodeManager, times(2)).findEpisodesByPodcastOrderedFlow(any()) confirms a re-subscription, not that the sort type drove it — it passes even with a wrong distinctUntilChangedBy key. The negative case (a podcast differing only in e.g. latestEpisodeUuid must not re-subscribe) is the actual regression guard for the fix in the previous round, and is still missing.	TvPodcastDetailsViewModelTest.kt:157
4	podcast_no_episodes_all_archived is a %d string, not a plural → "All 1 episodes of this podcast have been archived". The sibling uses plurals/tv_playlist_all_archived. Needs a new plural, which conflicts with the "no strings added" checkbox — your call.	TvPodcastDetailsScreen.kt:304
5	AllEpisodesArchived still duplicated byte-for-byte (modulo the string) across both screens — belongs in TvEpisodeListControls.kt with a text: String param, given the PR's premise is exactly this kind of extraction.	TvPodcastDetailsScreen.kt:295 / TvPlaylistDetailsScreen.kt:199
8	tv_playlists → tv_preferences isn't data-preserving (keys unchanged, only the file moved), and the orphaned tv_playlists.xml is no longer wiped by clearAll(). Non-issue if nothing shipped.	TvPreferences.kt:14
—	No @Preview for the newly-shared TvArchivedFilterButton / TvSortButton, though the checklist claims preview coverage. TvPreferences still shares the …pocketcasts.preferences package name with :modules:services:preferences, so TvSignOutManager imports Settings and TvPreferences from two modules under one package (…tv.preferences reads better).

Verified in passing

• No dangling references to the deleted TvPlaylistPreferences anywhere in the tree; the playlist VM/test and TvSignOutManager/test all moved cleanly to the renamed methods.
• EpisodesSortTypeLabel.kt matches PlaylistEpisodeSortTypeLabel.kt exactly in module, package, annotations and shape, and is exhaustive over all six entries with no else.
• No unused imports in any changed file (the IR alias removal from TvPlaylistDetailsScreen from round 1 still holds; the podcast screen's Spacer/width/fillMaxHeight/widthIn/MaterialTheme/TextAlign are all still referenced).

CI

Still no build, lint, or test job on this PR, and ./gradlew is blocked in this review environment (the command needs approval I don't have), so I could not run spotlessCheck or the tv unit tests myself — everything above is from reading the code. Worth getting a real build on this before merge, since nothing has compiled the branch in any of the four review rounds.

Recommendation: the two things I'd asked for before merge (A and F) are done. E is the only remaining item I'd genuinely push for — the distinctUntilChangedBy from round 2 is the kind of optimisation that silently regresses, and nothing pins it. Everything else is optional polish.
· feat/tv-podcast-details-sort-archived

[geekygecko]

Do we need this preference class? We have this on the podcast entity.

[sztomek]

Thanks @geekygecko — on the TvPreferences question: it's a deliberate split rather than a duplicate of the entity.

• Sort order does go on the synced Podcast entity (changeSortType → updateEpisodesSortTypeBlocking), exactly as you'd expect — that's shared with mobile.
• The archived filter is the one that's local (TvPreferences.isPodcastShowingArchived). The entity's showArchived is a synced field, so writing the TV toggle there would flip "show archived" on the user's phone too. The commit that introduced this ("Store the TV podcast archived filter locally instead of on the synced podcast") was specifically to keep the TV's view-filter from syncing back — it's a per-device UI preference, not user data.

So TvPreferences earns its place for that one setting; happy to reconsider if you'd rather the TV filter did sync to the entity (that's a product call). One fair follow-up the review bot also raised: the class currently lives in the …pocketcasts.preferences package shared with :services:preferences, so …tv.preferences would read cleaner — I can move it if we keep the class.

[claude]

Review (re-review after push — base is now main)

• Read the full diff vs origin/main
• Determine what changed since the previous review
• Re-check the outstanding items (B, C, E, 4, 5, 8)
• Look for new issues

View job run · feat/tv-podcast-details-sort-archived

[sztomek]

Round-6 bot review — treatment (rebased onto main first now that #5683 merged; the conflicts were the sort/archived-control extraction vs. main's 5683 treatment — resolved to keep both: the extracted TvEpisodeListControls and main's Caption/TvEmptyState/RowImageWidth changes, and the all-archived branch now uses the shared TvEmptyState for the truly-empty case while keeping AllEpisodesArchived for the filtered case).

Fixed — E (the one you'd push for): added a non-sort podcast change does not re-run the episodes query. It emits a podcast differing only in title (same episodesSortType) and asserts findEpisodesByPodcastOrderedFlow is called once — so distinctUntilChangedBy { it.episodesSortType } is now pinned; the test goes red (times 2) the moment that key regresses. The existing times(2) sort-change test stays as the positive case.

Deferring, with reasons (all the optional items):

• B — podcastFlow collected twice. Real (two Room executions + two invalidation observers), but shareIn inside the flow { } builder has enough lifecycle subtlety (scope, replay, WhileSubscribed interplay with the outer stateIn) that I'd want a device check rather than land it blind; it's a contained follow-up.
• C — intermediate (new podcast, old episodes) emission. You confirmed it self-corrects in a frame and doesn't defeat the scroll reset; cosmetic, and it collapses together with B.
• 4 — podcast_no_episodes_all_archived is %d, not a plural ("All 1 episodes"). Genuine grammar bug, but the fix adds a new plural string (GlotPress) against the PR's no-new-strings premise — flagging for your call rather than sneaking a string in.
• 5 — AllEpisodesArchived duplicated across both screens. Right that it belongs in TvEpisodeListControls with a text param; deferring only to keep this rebase off a third shared screen, and it pairs naturally with Switch to using issue forms #4 (the shared version can take a plural).
• G — unguarded scroll reset. You noted it's arguably better here (it makes the initial-focus request reachable after a restore); I read it the same way, so leaving it as the intentional choice.
• H — hasRequestedInitialFocus = episodes.isEmpty() reads oddly. It mirrors the sibling screen and is covered by tests; skipping the clarifying comment to stay consistent with it (and the house preference for terse code).
• 8 — tv_playlists → tv_preferences file rename + orphaned file. Non-issue pre-release (nothing shipped to wipe); noted for whenever storage compat matters.

Validation: spotlessCheck --rerun-tasks green (observed), :tv:compileDebugKotlin + :tv:testDebugUnitTest green.

[sztomek]

Fixed #4 (the All 1 episodes plural) after all.

podcast_no_episodes_all_archived turned out to be shared with mobile (PodcastAdapter.kt:541) and already translated into ~20 locales, so converting it in place would have broken the mobile call site and reset its translations. Instead I added a new TV-specific plural tv_podcast_all_archived mirroring the sibling tv_playlist_all_archived:

<plurals name="tv_podcast_all_archived">
    <item quantity="one">The only episode of this podcast has been archived.</item>
    <item quantity="other">All %1$d episodes of this podcast have been archived.</item>
</plurals>

and AllEpisodesArchived now uses pluralStringResource(...), so a 1-archived podcast reads "The only episode of this podcast has been archived." English-only per the localization guidelines; GlotPress will pull the translations.

Validation: :tv:compileDebugKotlin + :tv:testDebugUnitTest green, spotlessCheck --rerun-tasks green.

[claude]

Claude finished @sztomek's task in 3m 50s —— View job

---

Review (round 6 — rebased onto main, plural + negative test landed)

• Read the full diff vs origin/main
• Determine what changed since the previous review
• Re-check the outstanding items (B, C, E, 4, 5, 8)
• Look for new issues

The rebase onto main (post-#5683) is clean — 8 commits, 536/204 across 13 files, git merge-base HEAD origin/main is the main tip. The conflict resolution you described is what's in the tree: the extracted TvEpisodeListControls and main's TvEmptyState/Caption changes both survived, with the truly-empty case now going through the shared TvEmptyState and the filtered case keeping AllEpisodesArchived.

Fixed this round

#	Item	Verified
E	Negative regression guard for distinctUntilChangedBy	TvPodcastDetailsViewModelTest.kt:161-180 — emits a podcast differing only in title and asserts times(1). This is the assertion that goes red if the key regresses. 👍
4	All 1 episodes … grammar	plurals/tv_podcast_all_archived + pluralStringResource at TvPodcastDetailsScreen.kt:309

Your reasoning on #4 checks out — podcast_no_episodes_all_archived is used by PodcastAdapter.kt and is present in 19 values-* locale files, so converting it in place really would have broken mobile and reset those translations. A new TV-specific plural mirroring tv_playlist_all_archived was the right call.

Also re-verified, since the design leans on them: EpisodeDao's findByPodcastOrder*Flow queries have no archived predicate and no LIMIT (EpisodeDao.kt:200-244), so archivedEpisodeCount = episodes.count(PodcastEpisode::isArchived) counts the full set; and EpisodeManagerImpl.findEpisodesByPodcastOrderedFlow dispatches purely on podcast.episodesSortType, so distinctUntilChangedBy { it.episodesSortType } is the complete key. The three-way branch is also sound — AllEpisodesArchived is only reachable when episodes.isEmpty() && archivedEpisodeCount > 0, which implies isShowingArchived == false, so it can never claim "all archived" while archived episodes are on screen.

TvPlaylistDetailsScreen's accidental duplicate LaunchedEffect that I flagged last round is gone (fixed on main), and there are no unused imports in any changed file.

---

New this round

Both are minor; neither is a blocker.

I. TvPreferences keys are never pruned 🟢

TvPreferences.kt:36-38 writes one key per podcast/playlist UUID (show_archived_podcast_<uuid>) and nothing ever removes them — not on unfollow, not on playlist delete. tv_preferences.xml therefore grows monotonically for the life of the install (bounded only by how many things the user has toggled, and wiped only by clearAll() on sign-out). Not worth fixing now; worth knowing before more per-UUID flags get added to this class, since that's the point where it stops being negligible.

J. The initial preference read is a synchronous disk read on the main thread 🟢

TvPodcastDetailsViewModel.kt:49:

private val isShowingArchivedFlow = MutableStateFlow(preferences.isPodcastShowingArchived(podcastUuid))

This is a property initialiser, so it runs during hiltViewModel() construction — i.e. on the main thread during composition — and SharedPreferences.getBoolean blocks on the prefs-file load. A StrictMode DiskReadViolation in the worst case (first access after process start). Exactly mirrors TvPlaylistDetailsViewModel.kt:40, so it's pre-existing rather than introduced, and the file is tiny — flagging it only because the new podcast call site doubles the surface.

Asymmetry worth being aware of (not a bug)

The podcast screen focuses index 0 unconditionally, while the playlist screen restores focus to initialFocusIndex (the previously-first-visible item, TvPlaylistDetailsScreen.kt:160-166). That's internally consistent for the podcast screen — its unguarded scrollToItem(0) (previous round's G) always puts index 0 at the top anyway — so the two decisions hang together. Just noting the pair diverges as a set, in case one screen later gets "fixed" toward the other.

---

Already raised inline on this push

Both posted as inline comments, repeated here for the summary:

• TvSortButton inlines the icon-button colours (TvEpisodeListControls.kt:107-112) instead of calling TvButtonDefaults.iconButtonColors(), which the deleted SortDropdownButton used. I diffed them: TvButtonDefaults.kt:44-49 is byte-for-byte those four values, so there's no visual change today — but the shared design token is now duplicated and will silently diverge the next time TvButtonDefaults changes.
• changeSortType's uiState.value as? Loaded ?: return gate (TvPodcastDetailsViewModel.kt:86) buys nothing and adds a silent no-op path: PodcastManagerImpl.updateEpisodesSortTypeBlocking (line 562-563) forwards only podcast.uuid to a targeted UPDATE … WHERE uuid = :uuid, so the entity is unused and the already-injected podcastUuid suffices. Since stateIn uses WhileSubscribed(replayExpiration = ZERO), the state reverts to Loading once collection stops, making the early return a path where a tap is dropped.

---

Deferrals — agreed, with one note

Your rationale on B, C, G, H and 8 all reads right to me:

• B/C — agreed they collapse together, and C is confirmed benign for the scroll reset: LazyListState.scrollToItem routes through requestPositionAndForgetLastKnownKey, which drops the remembered first-visible key, so the reordered list arriving a frame later can't re-anchor the viewport onto the old first episode.
• G — agreed it's the better behaviour here specifically, because the focus effect waits on visibleItemsInfo.any { it.index == 0 } and would never resolve if the list restored at index 40.
• H — fine to mirror the sibling. For the record the flag is load-bearing: mutableStateOf(episodes.isEmpty()) is what stops "show archived" on an all-archived podcast from yanking focus off the dropdown.
• 8 — non-issue pre-release, agreed.

5 (AllEpisodesArchived duplicated across both screens) is the one I'd still nudge on, and it's cheaper now than it was: both copies are structurally identical and both now take a plural, so the shared version is text: String and two call sites. Entirely optional.

Device checks still worth doing before merge

Neither is provable by reading, and neither is covered by the testing steps:

1. Focus after hiding archived on an all-archived podcast. Focus an episode, then hide archived. The focused node leaves composition, hasRequestedInitialFocus is already true, and nothing in the AllEpisodesArchived branch claims focus — so the first D-pad press relies on Compose's focus search rather than moving from a known anchor. Testing step 5 only covers dropdown → dropdown.
2. The intermediate (new podcast, old episodes) emission on a sort change (C) — the only path where the two combine sources are observably out of step, and slow TV hardware is where it'd show.

CI

Still no build, lint, or test job on this PR — CI shows only Validate Gradle Wrapper ×2 plus this review job. ./gradlew is blocked in this environment, so I could not run spotlessCheck or the tv tests myself; I'm taking your reported green :tv:compileDebugKotlin / :tv:testDebugUnitTest / spotlessCheck at face value, but nothing in CI has compiled this branch across six review rounds.

Recommendation: E and 4 — the two items I'd pushed for — are done, and the code reads correct. The two inline comments are small cleanups worth taking; everything else is optional.
· feat/tv-podcast-details-sort-archived

[sztomek]

Both of these are right — fixed.

Inlined icon-button colours in the extraction. Good catch — TvSortButton had copy-pasted the four IconButtonDefaults.colors(...) values that are exactly TvButtonDefaults.iconButtonColors(). Swapped it back to the shared helper; IconButtonDefaults, TvColors and Color all became unused in this file (no other references), so removed all three imports. spotlessCheck --rerun-tasks is green.

changeSortType gate. Confirmed PodcastManagerImpl.updateEpisodesSortTypeBlocking only forwards podcast.uuid to the DAO's targeted UPDATE … WHERE uuid, so the (uiState.value as? Loaded)?.podcast ?: return read was dead weight that could silently drop a tap (state reverts to Loading under WhileSubscribed(replayExpiration = ZERO)). Dropped it and pass Podcast(uuid = podcastUuid) straight through using the constructor arg. Kept it contained in the TV VM rather than adding a uuid overload to the shared PodcastManager; happy to switch to the overload if you'd prefer not to fabricate the entity. Updated the persist test to verify against Podcast(uuid = "podcast-uuid") accordingly.

:tv:compileDebugKotlin + :tv:testDebugUnitTest green.

[claude]

Claude finished @sztomek's task in 3m 2s —— View job

---

Review (round 7 — after the two inline-comment fixes)

• Read the full diff vs origin/main
• Verify the two inline fixes (TvButtonDefaults.iconButtonColors(), changeSortType gate)
• Re-check outstanding items (B, C, 5, 8, I, J)
• Look for new issues

Both fixes landed and both are correct. Diff is 527/204 across 13 files, 8 commits, git merge-base HEAD origin/main is the main tip.

Verified fixed

Item	Verified
Inlined icon-button colours	TvEpisodeListControls.kt:99 — colors = TvButtonDefaults.iconButtonColors(). IconButtonDefaults, TvColors and Color are all gone from the import list, and I grepped the file: every remaining import is referenced, so no dead imports.
changeSortType gate	TvPodcastDetailsViewModel.kt:85-89 — the uiState.value as? Loaded read is gone; Podcast(uuid = podcastUuid) goes straight through. PodcastManagerImpl.kt:562-564 confirms only podcast.uuid reaches the DAO.

I also checked the thing that would have made the updated test flaky, since verify on a fabricated entity depends on structural equality: every Date field on Podcast defaults to null (Podcast.kt:34,47,48,53,55,57,59,63,66,79,81,83,85,86,89,91,93,95 — no = Date() defaults anywhere). So Podcast(uuid = "podcast-uuid") built in the test compares equal to the one built in the view model, and TvPodcastDetailsViewModelTest.kt:194 isn't time-sensitive. 👍

Also re-confirmed on this push: no dangling references to TvPlaylistPreferences / tv_playlists anywhere in the tree; TvPlaylistDetailsScreen's imports are all still live after the extraction (TvButtonDefaults at line 344, TvTextStyles at 287/327/332/337); the strings.xml diff is exactly the four lines of the new plural, mirroring tv_playlist_all_archived above it; and AllEpisodesArchived remains reachable only when episodes.isEmpty() && archivedEpisodeCount > 0, which implies isShowingArchived == false.

---

One follow-on note on the fabricated entity

Not a defect, just the reason the overload you offered is the sturdier option: the method immediately above the one you're calling in the same file does use more than uuid.

PodcastManagerImpl.kt:555-560:

override fun updateEffectsBlocking(podcast: Podcast, effects: PlaybackEffects) {
    podcastDao.updateEffectsBlocking(…, podcast.uuid)
    launch { updateTrimMode(podcast, effects.trimMode) }   // <- whole entity
}

So "pass a synthetic Podcast to a PodcastManager method" is safe for updateEpisodesSortTypeBlocking specifically, and nothing at the signature level distinguishes the two. It's correct today and I verified it; the risk is purely that a future change to the impl silently starts reading a field that's a default rather than the real value. Either the uuid overload on PodcastManager or a one-line comment at the call site covers it. Your call — I'd leave it if you'd rather not touch the shared interface.

Nit (new, from the extraction)

TvEpisodeListControls.kt:48 — the now-shared TvArchivedFilterButton styles its label with TvTextStyles.PlaylistCardCaption, a playlist-named token, on a component the podcast screen also uses. The neutral TvTextStyles.Caption is 17sp/w510 vs PlaylistCardCaption's 15sp, so swapping would be a visual change — this is naming only, and leaving it (or renaming the token separately) are both fine.

---

Still outstanding (all optional, unchanged)

	Item	Where
B	podcastFlow collected twice (by combine and by episodesFlow) → two Room executions + two invalidation observers for the same row. Agreed with deferring given the shareIn-inside-flow {} lifecycle subtlety.	TvPodcastDetailsViewModel.kt:63-68
C	combine emits one intermediate Loaded pairing the new podcast with the old episode order. Benign for the scroll reset (scrollToItem goes through requestPositionAndForgetLastKnownKey, so the reordered list can't re-anchor). Collapses with B.	TvPodcastDetailsViewModel.kt:68-75
5	AllEpisodesArchived still duplicated across both screens — now that both take a plural, the shared version is text: String + two call sites. Cheapest it's ever going to be, but genuinely optional.	TvPodcastDetailsScreen.kt:301 / TvPlaylistDetailsScreen.kt:~215
8	tv_playlists → tv_preferences isn't data-preserving and the orphan isn't wiped by clearAll(). Non-issue pre-release.	TvPreferences.kt:14
I	Per-UUID keys are never pruned (not on unfollow, not on playlist delete), so tv_preferences.xml grows monotonically until sign-out. Negligible at two flags; worth knowing before more get added.	TvPreferences.kt:37-39
J	MutableStateFlow(preferences.isPodcastShowingArchived(…)) is a property initialiser, so the SharedPreferences disk read happens on the main thread during hiltViewModel(). Pre-existing (mirrors TvPlaylistDetailsViewModel).	TvPodcastDetailsViewModel.kt:49
—	No @Preview for the newly-shared TvArchivedFilterButton / TvSortButton; TvPreferences still shares the …pocketcasts.preferences package name with :modules:services:preferences, so TvSignOutManager imports Settings and TvPreferences from two modules under one package. You already offered to move it to …tv.preferences — that one seems worth taking while the class is new.

Device checks still worth doing

Neither is provable by reading and neither is in the testing steps:

1. Focus after hiding archived on an all-archived podcast. Focus an episode, then hide archived: the focused node leaves composition, hasRequestedInitialFocus is already true, and the AllEpisodesArchived branch claims nothing — so the next D-pad press depends on Compose's focus search rather than a known anchor. Step 5 only covers dropdown → dropdown.
2. The intermediate (new podcast, old episodes) emission on a sort change (C) — the one path where the two combine sources are observably out of step, and slow TV hardware is where it'd surface.

CI

Still no build, lint, or test job on this PR (CI shows only Validate Gradle Wrapper ×2 plus this review job), and ./gradlew remains blocked in this environment, so I could not run spotlessCheck or :tv:testDebugUnitTest myself. Everything above is from reading the code and grepping the tree; I'm taking your reported greens at face value. Seven review rounds in, nothing in CI has compiled this branch — worth getting one real build before merge.

Recommendation: everything I'd pushed for is now done and the code reads correct. The remaining items are polish; the …tv.preferences package move is the only one I'd bother with in this PR.
· feat/tv-podcast-details-sort-archived