Skip to content

[TV] Search suggestions and history - #5746

Draft
sztomek wants to merge 9 commits into
fix/tv-search-network-crashfrom
feat/tv-search-parity
Draft

[TV] Search suggestions and history#5746
sztomek wants to merge 9 commits into
fix/tv-search-network-crashfrom
feat/tv-search-parity

Conversation

@sztomek

@sztomek sztomek commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

Description

Brings the Android TV Search screen up to parity with the Apple TV app (pocket-casts-ios/Pocket Casts TV App/UI/Search), closing a set of gaps found in a behavioural audit. Stacked on #5745 (the combined-search crash fix) — review/merge that first; this PR's diff excludes it.

What changed

Area Before (Android TV) After (Apple TV parity)
Debounce 2000ms (shared phone remote-config default) 300ms TV constant — matches iOS, far snappier
Empty results generic "No results" "No results for "term"" (shows the query)
Loading bare spinner spinner + "Searching…" label (string existed, was unused)
Scope on clear reset to Top Results preserved (matches iOS)
Field prompt "Search" "Podcasts, shows, authors"
Search flow single combinedSearch (all-at-once) two-phase: predictive (autoCompleteSearch) → fast podcasts + term suggestions, then full search fills in episodes
Suggestions none term suggestions while typing, rendered above the keyboard; tap to fill
History never saved/shown (manager only used to wipe on sign-out) saved on finishing a search / picking a suggestion + "Recent searches" row in the idle area
Video episodes has_video dropped by the DTO parsed through DTO → model, driving a Featured video row
Top Results Podcasts → Episodes (vertical) Featured (video) → Episodes → Podcasts horizontal carousels (matches SearchTopResultsView)
Podcast ordering subscribed-first server-relevance first, subscribed fill gaps (matches iOS) — see flag below

Shared servers/model change: CombinedResult.EpisodeResult now parses has_video, and ImprovedSearchResultItem.EpisodeItem carries hasVideo (additive, defaults false — benefits phone too).

⚠️ Needs design approval

There is an ongoing conversation about the UX, see slack: p1786735532787109-slack-C0ATWH7BNH3

Fixes POC-848 https://linear.app/a8c/issue/POC-848/search-suggestions-and-history

Notes / follow-ups

  • Suggestions can't live inside the system keyboard — Android exposes no API to inject candidates into a third-party IME (and a custom keyboard was previously rejected by design), so they render in-app just above the keyboard.
  • Episodes scope still shows a flat grid of all episodes; splitting out the Featured video row there too is a small follow-up.
  • Focus-restore after returning from a detail now lands on the first Top-Results section (was the Podcasts row) — coarse, not a bug.
  • Device UI verification pending: the shared TV emulator is hard to drive precisely and isn't logged in; the logic + crash fix are unit-tested and all module builds/tests are green, but the interactive Compose UI (suggestions/history/carousels focus) should be eyeballed on a real device. Screenshots to follow.

Testing Instructions

  1. Merge/stack on [TV] Fix search failing on unknown combined-search result types #5745 so search works (otherwise network-type results still error).
  2. ./gradlew :tv:installDebug. Open Search, type a query:
    • term suggestions appear above the keyboard while typing; tapping one fills + searches.
    • podcasts appear quickly, then episodes fill in.
    • Top Results shows Featured (video) → Episodes → Podcasts carousels.
    • an empty query returns to the idle browse; if you've searched before, a Recent searches row appears.
    • a no-match query shows "No results for "…"".
  3. Unit tests: ./gradlew :tv:testDebugUnitTest :modules:services:servers:testDebugUnitTest :modules:services:repositories:testDebugUnitTest.

Screenshots or Screencast

Screen_recording_20260814_212101.mp4

Checklist

  • If this is a user-facing change, I have added an entry in CHANGELOG.md — N/A (TV, pre-release)
  • Ensure the linter passes (./gradlew spotlessApply)
  • I have considered whether it makes sense to add tests for my changes
  • All strings that need to be localized are in localization
  • 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 — N/A (no analytics change)

@claude

claude Bot commented Aug 14, 2026

Copy link
Copy Markdown

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


Review: [TV] Search parity with Apple TV

  • Gather context (diff vs fix/tv-search-network-crash, read changed files)
  • Review ViewModel / search flow logic
  • Review Compose UI changes
  • Review shared servers / model / repositories changes
  • Post review (6 inline comments + summary below)

Nice piece of work — the parity table in the description made this easy to follow, and calling out the podcast-ordering reversal for a decision rather than burying it is exactly right. The hasVideo plumbing is minimal and additive, the debounce constant is a clear win, and the test diff tracks the behaviour changes honestly (renaming "subscribed podcasts lead the results" rather than deleting it).

Most of what follows falls out of one thing: the search is now two-phase, and a few places still assume it's atomic.

Blocking-ish

1. Unguarded suspend calls can crash the appTvSearchViewModel.kt:122
findSubscribedFlow(term).first() moved out of the try during the refactor. viewModelScope has no exception handler, so a throw is uncaught rather than TvSearchState.Error. Same for refreshHistory() in init (L106) and searchHistoryManager.add(...) in saveSearchTerm (L169). Every other suspend call in the file is wrapped, so this looks accidental. Fix this →

2. Transient false "No results for …"TvSearchViewModel.kt:135
The early publish hard-codes episodes = emptyList(). Because the scope is no longer reset on clear, a user parked on the Episodes filter gets the full no-results empty state mid-search, then a populated grid a round-trip later. Suggest an isPartial flag on Results that the empty branches treat as "still loading". Details inline.

3. The two phases are serialisedTvSearchViewModel.kt:122-139
autoCompleteSearch is fully awaited before combinedSearch is issued, so episodes now arrive one extra RTT later than on the base branch. Overlapping them with async preserves the early-podcast win without regressing full-result latency.

Worth fixing before merge

4. Three dead imports in TvSearchEpisodeRow.kt after deleting the TvSearchEpisodeRow composable — FocusRequester (L21), focusRequester (L22), TvEpisodeListItemContainer (L33). None are referenced any more. Fix this →

5. Preview coverage — the checklist claims previews cover the new components, but TvSearchScreenPreview still only renders Idle with an empty history. Nothing exercises the suggestions list, the Recent-searches row, the Featured carousel, or the new TvSearchLoading label — which is a shame given the interactive bits are the ones you can't verify on the emulator. Fix this →

6. Redundant truncationSearchHistoryManagerImpl.add() already truncates to 20, so truncateHistory(SEARCH_HISTORY_LIMIT) and the constant duplicate repository behaviour, and the new test asserts on the duplicate. Inline.

7. isFollowed is now wrong for predictive podcastsImprovedSearchManagerImpl.autoCompleteSearch never populates isSubscribed, and predictive entries now win the distinctBy, dropping the correctly-flagged local copy. Invisible today (TvPodcastTile ignores the field) but it's latent. Inline.

Focus / UX to verify on device

8. Focus black hole after picking a suggestion — the focused tile is removed from composition in the same frame with no explicit focus target. Other restore sites here all use a FocusRequester; this one doesn't. Inline.

9. Rows inserted above the focus — when the full search lands, Featured and Episodes carousels appear above the podcasts row the user may already be scrolling. That's the price of progressive + Apple's row order; just flagging it as the thing to watch when you get a device.

On your open question — podcast ordering

Worth noting that the code doesn't quite do what the description says. The precedence is predictivePodcastsremotePodcastslocalPodcasts, so the visible order is dominated by the autocomplete endpoint's relevance, not combinedSearch's. That's arguably the better choice — the row doesn't reshuffle when the full results land — but it isn't "server-relevance first" in the sense the table implies, so it's worth stating explicitly when the team decides.

For what it's worth: subscribed-first is a stronger default on a 10-foot UI, where scrolling is expensive and the shows you follow are the ones you're most likely reaching for. If you keep remote-first, a "Your shows" section would recover most of that.

Smaller notes

  • hasVideo: Boolean = false with Moshi codegen handles an absent key fine, but an explicit "has_video": null will throw. Boolean? + ?: false is cheap insurance if the server's contract isn't guaranteed.
  • Suggestions are only cleared on an empty query, so re-entering the field after a completed search briefly shows stale suggestions for the previous term.
  • Saving on field blur persists abandoned and typo'd terms — and this writes into the shared search_history table that the phone app reads, so TV noise shows up in phone recents. (The term column is uniquely indexed, so the key = { it } on the Recent searches row is safe — I checked.)
  • findAll(showFolders = false) loads podcast and episode history rows just to filterIsInstance them away.
  • TvSearchLoading has no spacing between the spinner and the "Searching…" label.

I did not build or run the test suite in this environment; findings are from reading the diff and surrounding code.
• branch feat/tv-search-parity

@dangermattic

Copy link
Copy Markdown
Collaborator
1 Message
📖 This PR is still a Draft: some checks will be skipped.

Generated by 🚫 Danger

Comment thread tv/src/main/java/au/com/shiftyjelly/pocketcasts/search/TvSearchViewModel.kt Outdated
Comment thread tv/src/main/java/au/com/shiftyjelly/pocketcasts/search/TvSearchViewModel.kt Outdated
Comment thread tv/src/main/java/au/com/shiftyjelly/pocketcasts/search/TvSearchViewModel.kt Outdated
Comment thread tv/src/main/java/au/com/shiftyjelly/pocketcasts/search/TvSearchScreen.kt Outdated
@sztomek sztomek added this to the 8.19 milestone Aug 14, 2026
@sztomek sztomek changed the title [TV] Search parity with Apple TV [TV] Search suggestions and history Aug 14, 2026
@sztomek
sztomek force-pushed the feat/tv-search-parity branch from 69e600c to 489431e Compare August 14, 2026 19:38
@wpmobilebot wpmobilebot modified the milestones: 8.19, 8.20 Aug 17, 2026
@wpmobilebot

Copy link
Copy Markdown
Collaborator

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants