feat: add on-demand transcript generation - #5762
Conversation
Co-authored-by: Cursor <cursoragent@cursor.com>
|
Claude finished @stefanosala's task in 6m 28s —— View job Code review: on-demand transcript generation
Nice shape overall — the feature is properly flag-gated, Blocking1. The transcript page gets stuck on an indefinite spinner — Once generation ends in Reachable in both hosts as written: 2. Polling costs ~60 network requests per episode view — Each of the 20 attempts does Worth fixing before merge3. Error mapping surfaces the wrong copy —
4. Proto enum values pollute the package namespace — Proto3 scopes top-level enum values to the package, so 5. Repository has no behavioural tests — The test file covers only the Smaller items
Not verifiedI reviewed statically only — I didn't build the modules or run the test suites, so I can't confirm |
Generated by 🚫 Danger |
|
|
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
Adds a production-off, feature-flagged “on-demand transcript generation” path for eligible (Plus/Patron) listeners when no transcript exists, including a new sync API endpoint, repository orchestration, UI states/copy, and polling/refresh logic while the transcript screen is visible.
Changes:
- Introduces a new
ON_DEMAND_TRANSCRIPTSfeature flag and protobuf-backed sync endpoint (/user/transcript/on_demand) for requesting generation. - Adds
OnDemandTranscriptRepository+ ViewModel logic to request once, then poll refresh (15s / max 20 attempts) and surface new UI states (Generating / Delayed / Failed / Unavailable). - Updates episode/player surfaces to keep Transcript entry points available for eligible users even when a transcript is initially missing, plus adds/updates tests.
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| modules/services/utils/src/main/java/au/com/shiftyjelly/pocketcasts/utils/featureflag/Feature.kt | Adds ON_DEMAND_TRANSCRIPTS feature flag definition. |
| modules/services/servers/src/main/java/au/com/shiftyjelly/pocketcasts/servers/sync/SyncServiceManager.kt | Adds manager wrapper to call on-demand transcript sync endpoint. |
| modules/services/servers/src/main/java/au/com/shiftyjelly/pocketcasts/servers/sync/SyncService.kt | Adds Retrofit API method for /user/transcript/on_demand. |
| modules/services/servers/src/main/java/au/com/shiftyjelly/pocketcasts/servers/ShowNotesServiceManager.kt | Exposes show-notes download helper used by transcript metadata refresh. |
| modules/services/repositories/src/main/java/au/com/shiftyjelly/pocketcasts/repositories/transcript/OnDemandTranscriptRepository.kt | New repository to request generation and refresh local metadata. |
| modules/services/repositories/src/test/java/au/com/shiftyjelly/pocketcasts/repositories/transcript/OnDemandTranscriptRepositoryTest.kt | Unit tests for HTTP-status → domain outcome mapping. |
| modules/services/repositories/src/main/java/au/com/shiftyjelly/pocketcasts/repositories/shownotes/ShowNotesManager.kt | Adds refreshTranscriptMetadata used during polling refresh. |
| modules/services/repositories/src/main/java/au/com/shiftyjelly/pocketcasts/repositories/di/RepositoryModule.kt | Wires OnDemandTranscriptRepository into DI graph. |
| modules/services/protobuf/src/main/proto/sync_api.proto | Adds protobuf request/response + enums for on-demand transcript generation. |
| modules/services/localization/src/main/res/values/strings.xml | Adds new listener-facing strings for generation states (prototype copy). |
| modules/features/transcripts/src/test/kotlin/au/com/shiftyjelly/pocketcasts/transcripts/TranscriptViewModelTest.kt | Adds ViewModel tests covering request-once and refresh/poll behavior. |
| modules/features/transcripts/src/main/kotlin/au/com/shiftyjelly/pocketcasts/transcripts/ui/TranscriptPage.kt | Adds UI for new transcript generation states + lifecycle start/stop hooks. |
| modules/features/transcripts/src/main/kotlin/au/com/shiftyjelly/pocketcasts/transcripts/TranscriptViewModel.kt | Implements on-demand request + visible-only polling refresh and tracking events. |
| modules/features/transcripts/build.gradle.kts | Adds lifecycle-compose runtime dependency for LifecycleStartEffect. |
| modules/features/podcasts/src/main/java/au/com/shiftyjelly/pocketcasts/podcasts/view/episode/EpisodeFragmentViewModel.kt | Computes “can request on-demand transcript” eligibility for tab behavior. |
| modules/features/podcasts/src/main/java/au/com/shiftyjelly/pocketcasts/podcasts/view/episode/EpisodeFragment.kt | Keeps Transcript tab accessible for eligible users; adjusts tracking params. |
| modules/features/player/src/test/java/au/com/shiftyjelly/pocketcasts/player/viewmodel/ShelfViewModelTest.kt | Updates tests for new UserManager dependency. |
| modules/features/player/src/test/java/au/com/shiftyjelly/pocketcasts/player/viewmodel/ShelfSharedViewModelTest.kt | Adds tests for transcript availability gating with the new feature flag. |
| modules/features/player/src/main/java/au/com/shiftyjelly/pocketcasts/player/viewmodel/ShelfViewModel.kt | Marks transcript “available” when eligible for on-demand, even if missing. |
| modules/features/player/src/main/java/au/com/shiftyjelly/pocketcasts/player/viewmodel/ShelfSharedViewModel.kt | Same eligibility-based transcript availability in shared shelf UI state. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Prevent terminal-state re-entry and reduce polling work while improving outcome handling, previews, and behavioral coverage. Co-authored-by: Cursor <cursoragent@cursor.com>
Review follow-upFixed
Not fixed
Validation
Commit: 1cea818 |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 23 out of 23 changed files in this pull request and generated no new comments.
Suppressed comments (2)
modules/features/transcripts/src/main/kotlin/au/com/shiftyjelly/pocketcasts/transcripts/TranscriptViewModel.kt:270
- When returning to the transcript screen after polling has stopped (e.g.,
GenerationDelayed/other terminal generation states),loadTranscript()currently short-circuits andonScreenStarted()does not attempt to load an already-generated transcript from local DB. This can leave users stuck on “Check back later” even after the transcript becomes available, until the ViewModel is recreated.
Consider checking for an existing transcript once on screen start when in a terminal generation state (without re-requesting generation).
fun onScreenStarted() {
isScreenStarted = true
if (_uiState.value.transcriptState is TranscriptState.Generating) {
startGenerationRefresh()
}
modules/services/repositories/src/main/java/au/com/shiftyjelly/pocketcasts/repositories/transcript/OnDemandTranscriptRepository.kt:127
String.lowercase()is locale-sensitive, which can lead to inconsistent analytics values on some locales (e.g., Turkish locale casing). Analytics keys should be locale-independent.
private val OnDemandTranscriptReason.analyticsValue
get() = name.lowercase()
private val OnDemandTranscriptEnablement.analyticsValue
get() = name.lowercase()
Linear: PCDROID-729
Description
Adds on-demand transcript generation for Plus and Patron listeners when an episode has no existing transcript. The client checks for creator-provided content first, makes one authenticated request, and refreshes only while visible every 15 seconds for at most 20 attempts/five foreground minutes. It then stops in a check-later state. The feature remains production-off.
Fixes: PCDROID-729
Testing Instructions
Screenshots or Screencast
Required before moving out of draft because this changes listener-facing states. Current copy is prototype copy pending Design review.
Verification
./gradlew spotlessCheckChecklist
./gradlew spotlessApplyto automatically apply formatting/linting)modules/services/localization/src/main/res/values/strings.xmlI have tested any UI changes...
Made with Cursor