feat(catalog): fill missing PI episodes from the publisher feed and notify from RSS - #971
Merged
Conversation
Add a one-time Podcast Info opt-in that keeps PI ownership while refreshing missing episodes from the publisher feed on each open, with a top-bar pill for Missing episodes? / Fetching / Updated.
…I lags Keep PI+feed episode lists in PodcastRepository, promote feed tips for Home NEW, and poll the publisher feed for new-episode pushes when a notifying show is opted in.
ashwkun
added
user-impact-high
|
Warning Review limit reached
Next review available in: 46 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (8)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe PR adds Podcast Index episode supplements backed by Room and direct-feed synchronization. It merges feed-only episodes into catalog and podcast views, hydrates new-episode notifications, updates badges and playback, and changes the episode checker to use RSS-first processing with tests. ChangesEpisode supplement storage and feed integration
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related issues
Possibly related PRs
Important Pre-merge checks failedPlease resolve all errors before merging. Addressing warnings is optional. ❌ Failed checks (2 errors, 1 warning)
✅ Passed checks (6 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
7 tasks
ashwkun
added
user-impact-critical
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 40
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
feature/home/src/main/java/cx/aswin/boxlore/feature/home/HomeFeedYourShows.kt (1)
29-47: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winFreeze the skeleton row mode for one loading session.
The count starts at
0, which renders two rows. If Room then emits one through four subscriptions, the current code changes to one row beforeviewportReady. A later count above four changes it back to two rows.
feature/home/src/main/java/cx/aswin/boxlore/feature/home/HomeFeedYourShows.kt#L29-L47: Store an immutable one-row or two-row mode when loading starts. Do not derive the mode from later count emissions.feature/home/src/main/java/cx/aswin/boxlore/feature/home/components/HomeSkeleton.kt#L224-L227: Accept the fixed row mode instead of recalculating it fromsubscribedCount.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@feature/home/src/main/java/cx/aswin/boxlore/feature/home/HomeFeedYourShows.kt` around lines 29 - 47, Freeze the skeleton layout mode once per loading session instead of updating skeletonLayoutCount from later Room emissions: in HomeFeedYourShows.kt, initialize an immutable one-row or two-row mode when loading begins and pass it through YourShowsFeedContent. In HomeSkeleton.kt lines 224-227, accept that fixed mode and stop recalculating the row count from subscribedCount; update both affected call paths consistently.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/new-episode-check.yml:
- Around line 34-38: Update the “Test checker logic” step to run after “Install
Dependencies” and invoke the documented scripts/package.json
test:check-new-episodes npm script instead of calling Node directly, preserving
the existing test purpose.
In `@app/src/main/java/cx/aswin/boxlore/fcm/NewEpisodePushHydration.kt`:
- Around line 14-44: Update resolveLocalEpisode in
app/src/main/java/cx/aswin/boxlore/fcm/NewEpisodePushHydration.kt to accept the
pushed GUID and only promote a resolved episode matching that GUID or payload
enclosure URL; never promote an unrelated newer feed tip. In
app/src/main/java/cx/aswin/boxlore/fcm/BoxLoreFcmService.kt lines 105-110, pass
FcmPayloadParser.guid(data) into hydration. In
app/src/test/java/cx/aswin/boxlore/fcm/NewEpisodePushHydrationTest.kt lines
61-122, add regression coverage for an advanced feed, asserting the payload
episode is selected or the unrelated newer episode is not promoted.
In `@app/src/test/java/cx/aswin/boxlore/fcm/NewEpisodeFcmLogicTest.kt`:
- Around line 34-39: Extend durationMinutesPrefersLocalSeconds in
NewEpisodeFcmLogicTest to assert that nonnumeric and negative payload durations,
such as "invalid" and "-1", return 0 when passed to
NewEpisodeFcmLogic.durationMinutes, while preserving the existing assertions.
In `@app/src/test/java/cx/aswin/boxlore/fcm/NewEpisodePushHydrationTest.kt`:
- Around line 61-122: Extend the tests around
NewEpisodePushHydration.resolveLocalEpisode with an error-path case where
FakePort throws while resolving the newest tip. Use a subscribed podcast with no
cached enclosure match, assert the result is null, and verify the stored
latestEpisode remains unchanged rather than being overwritten.
In
`@core/catalog/src/main/java/cx/aswin/boxlore/core/catalog/PodcastRepositoryNetworkLookups.kt`:
- Around line 68-70: Update PodcastRepository.getEpisodeImpl so colliding
negative episode IDs use an owner-scoped episode reference or lookup contract
that preserves the supplement row’s Podcast Index podcastId instead of
unconditionally preferring getRssEpisode. Add a regression test covering RSS and
supplement rows with the same negative episodeId and asserting the
supplement-owned result is returned.
In
`@core/catalog/src/main/java/cx/aswin/boxlore/core/catalog/SubscriptionForegroundSync.kt`:
- Around line 144-155: Introduce a defaulted DirectFeedSyncSeams data class
containing loadOptedInIds, loadPodcastMeta, loadCachedFeedTip, resolveFeedTip,
saveDirectFeedLatest, and feedNetworkDelayMs, then replace those six parameters
in syncSubscribedLatestEpisodes with one holder parameter. Update the method to
read the grouped values from the holder while preserving existing defaults and
behavior, and adjust tests/callers to pass these values as named arguments on
DirectFeedSyncSeams.
- Around line 94-104: Update the resolveFeedTip callback in syncOneDirectFeedTip
to check meta.feedUrl before calling
episodeSupplementPort.resolveNewestTipFromFeed; skip resolution when the URL is
null or blank, and only pass a nonblank feed URL to the port instead of using
meta.feedUrl.orEmpty().
- Around line 106-127: Remove the existing latest-episode comparisons and early
returns from both saveLatest and saveDirectFeedLatest callbacks. Let
SubscriptionRepository.updateLatestEpisode perform the replacement check,
retaining markAsNew = true for saveDirectFeedLatest so new-episode marking
occurs only when the repository accepts the tip.
- Around line 274-277: Update syncTrackedFeedUrlsForOptedInNotifications,
promoteCachedDirectFeedTip, and syncOneDirectFeedTip so each catch block
rethrows CancellationException before logging other exceptions. Preserve the
existing logging and handling for non-cancellation failures, following the
pattern used by PodcastRepository.syncSubscriptions and
NewEpisodePushHydration.resolveLocalEpisode.
In
`@core/catalog/src/main/java/cx/aswin/boxlore/core/catalog/SubscriptionRepository.kt`:
- Around line 232-234: Update updateFirebaseSubscription to remove the existing
tracked_podcasts entry or its feedUrl child when isSubscribed is false,
including when setNotificationsEnabled passes feedUrl as null. Preserve the
current write behavior for active subscriptions, ensuring feedUrl remains
persisted only when notifications and opt-in are both enabled.
In
`@core/catalog/src/test/java/cx/aswin/boxlore/core/catalog/SubscriptionForegroundSyncTest.kt`:
- Around line 158-198: Rename
syncSubscribedLatestEpisodes_promotesCachedTipWithoutNetworkWhenCurrent to
reflect that it promotes the cached tip before performing a network refresh,
while preserving the existing networkCalls == 1 assertion. Only add a separate
no-network test if that behavior is intended and supported.
In
`@core/database/src/main/java/cx/aswin/boxlore/core/database/BoxLoreDatabaseMigrations.kt`:
- Around line 66-110: Add a v30-to-v31 regression test using MigrationTestHelper
and the 30.json schema snapshot, migrate the database to version 31, and query
both episode_supplements and episode_supplement_items to verify the migration
creates and exposes both tables successfully. Anchor the test to migrate30To31
and the existing database test conventions.
In
`@core/database/src/main/java/cx/aswin/boxlore/core/database/EpisodeSupplementDao.kt`:
- Around line 35-47: Update the KDoc reference in EpisodeSupplementDao.search to
link escapeForSqlLike from the core.rss package instead of core.catalog.
Preserve the existing SQL query and escaping guidance.
In `@core/model/src/main/java/cx/aswin/boxlore/core/model/Podcast.kt`:
- Around line 60-61: Update the core model module README using its existing
README template to document that rssHasNewEpisodes covers both true-RSS
freshness checks and Podcast Index direct-feed tip promotions. Keep the
documentation aligned with the shared freshness behavior implemented in Podcast.
In
`@core/rss/src/main/java/cx/aswin/boxlore/core/rss/EpisodeSupplementListMerge.kt`:
- Around line 28-31: Update matchesUniqueOrDatedTitle and the filtering logic at
core/rss/src/main/java/cx/aswin/boxlore/core/rss/EpisodeSupplementListMerge.kt:28-31
so titles match only when normalized titles are non-blank and publication dates
match whenever both episodes have dates; do not use a title alone as identity.
At
core/rss/src/main/java/cx/aswin/boxlore/core/rss/EpisodeSupplementListMerge.kt:45-56,
replace ID-only deduplication with the same PI/feed matching policy, including
audio URL and dated-title matching while preserving distinct PI and rss:
identities. Add regressions for distant-date same-title episodes and PI results
matching cached supplements by audio URL or dated title.
In
`@core/rss/src/main/java/cx/aswin/boxlore/core/rss/EpisodeSupplementMatcher.kt`:
- Around line 33-35: In EpisodeSupplementMatcher.kt, update the title-matching
logic to compute the normalized RSS title and only evaluate baseline title
matches when that value is non-empty, preventing blank or punctuation-only
titles from matching. In EpisodeSupplementMatcherTest.kt, add a regression test
covering blank or punctuation-only titles with different audio URLs and verify
they do not match.
In
`@core/rss/src/main/java/cx/aswin/boxlore/core/rss/EpisodeSupplementRepository.kt`:
- Around line 50-53: Update both catch paths in
core/rss/src/main/java/cx/aswin/boxlore/core/rss/EpisodeSupplementRepository.kt
at lines 50-53 and 95-98 to use a shared private failure-mapping helper. The
helper should log the exception details separately and return
EpisodeSupplementOutcome.Failure with the fixed user-facing message “Couldn't
load feed”; replace both inline error.message mappings with this helper so the
paths cannot diverge.
- Around line 201-224: The EpisodeSupplementRepository behavior is untested. Add
a JVM test class under src/test for EpisodeSupplementRepository using
MockWebServer to cover blank and rss: podcast ID require guards, HTTPS-only
rejection, stored feedUrl fallback, Failure mapping for both catch paths, and
addedCount computation; keep the tests hermetic and verify the existing success
behavior.
- Around line 226-235: Reduce persistParsedSupplement’s parameter count by
introducing a private metadata data class that groups podcastTitle,
podcastImageUrl, podcastGenre, and podcastArtist. Update both call sites to
construct and pass this metadata object, then use its fields when calling
EpisodeSupplementTipLogic.resolveNewestTip; keep EpisodeSupplementPort
signatures unchanged.
- Around line 143-155: Make resolveNewestTipFromFeed persist the refreshed
supplement and any unmatched tip item through one transactional DAO operation,
adding and reusing EpisodeSupplementDao.upsertSupplementWithItems rather than
separate writes. Document on EpisodeSupplementPort.resolveNewestTipFromFeed that
the method can throw, or consistently catch failures and return null like its
sibling methods; preserve the existing validation and fetch behavior.
In
`@core/rss/src/main/java/cx/aswin/boxlore/core/rss/EpisodeSupplementTipLogic.kt`:
- Around line 25-31: Update the metadata merging in the matched.copy call to
treat blank strings as absent for podcastImageUrl, podcastGenre, and
podcastArtist, matching the existing podcastTitle behavior. Preserve each
matched baseline value when the corresponding incoming metadata is null or
blank, while continuing to use non-blank incoming values.
In
`@core/rss/src/test/java/cx/aswin/boxlore/core/rss/EpisodeSupplementDisconnectLogicTest.kt`:
- Around line 48-57: Add a test alongside `empty baseline with a feed tip means
disconnect` in `EpisodeSupplementDisconnectLogicTest` that calls
`EpisodeSupplementDisconnectLogic.shouldOptIn` with `newestFeedPublishedDate`
set to a non-positive value and asserts `false`, covering the unknown feed-date
guard while preserving the existing behavior for valid feed dates.
In
`@feature/info/src/main/java/cx/aswin/boxlore/feature/info/PodcastInfoScreen.kt`:
- Around line 651-663: Reorder the Compose overlays so SnackbarHost is declared
after the Jump-To pill, ensuring snackbar content draws above the pill when both
are visible. Keep the existing SnackbarHost state, alignment, and padding
unchanged.
In
`@feature/info/src/main/java/cx/aswin/boxlore/feature/info/PodcastInfoSupplementSupport.kt`:
- Around line 181-190: Introduce a small SupplementPodcastMeta value holder
containing podcastTitle, podcastImageUrl, podcastGenre, and podcastArtist, then
update unionSearch, refreshMissingEpisodes, autoOptInOnSubscribeIfDisconnected,
and the related EpisodeSupplementPort calls to pass this holder instead of four
separate metadata parameters. Preserve the existing metadata values and behavior
while reducing each affected method signature to at most seven parameters.
- Around line 64-85: Bound the work performed by refreshMissingEpisodes so
opening a podcast page does not always fetch up to SUPPLEMENT_BASELINE_LIMIT
episodes and then reload the same display data. Reuse state.piEpisodes when it
covers the required feed window, or cache/gate the baseline refresh with a
minimum interval; when a baseline repository result is needed, pass it through
to reloadDisplayPage instead of issuing another request. Preserve refresh
behavior for eligible podcasts while avoiding repeated full catalog and RSS
fetches on every open.
- Around line 15-18: Add PodcastInfoSupplementSupportTest.kt under
feature/info/src/test using fake EpisodeSupplementPort and PodcastRepository
implementations. Cover chip-state resolution for hidden, offer, fetching, and
updated states; refresh/auto-opt-in outcomes Success, NoDisconnect, and Failure;
and unionSearch’s RSS short-circuit, preserving hermetic JVM-only tests.
- Around line 201-202: Update EpisodeSupplementMergeLogic.unionSearchResults and
its call site to make supplement precedence explicit rather than relying on
positional argument order. Add a preferSupplements entry point or clearly named
preferred/additional parameters, and use it from the surrounding
supplement-search flow so duplicate IDs always retain supplement matches.
In
`@feature/info/src/main/java/cx/aswin/boxlore/feature/info/PodcastInfoUiModels.kt`:
- Around line 39-40: Update the KDoc for piEpisodes in the podcast UI model to
describe it as the accumulated episode list returned by the repository page,
including any merged cached feed-only extras, rather than as episodes before
supplement merge. Leave the property type and default value unchanged.
In
`@feature/info/src/main/java/cx/aswin/boxlore/feature/info/PodcastInfoViewModel.kt`:
- Around line 342-354: Update MissingEpisodesRefresh to return the reloaded
page’s sourceCount, then have loadMissingEpisodes at PodcastInfoViewModel.kt
lines 342-354 and the silent refresh-on-open block at lines 492-521 assign
currentOffset from that shared value after refreshMissingEpisodes completes;
preserve the existing state and subscription updates.
- Around line 487-521: Add JVM tests under feature/info/src/test for the new
PodcastInfoViewModel supplement behavior, using hermetic fakes for
PodcastRepository, SubscriptionRepository, and EpisodeSupplementPort. Cover chip
state transitions, silent refresh on open success, NoDisconnect fallback,
refresh failure fallback, and post-subscribe auto-opt-in including its
syncSubscriptions fallback, anchoring tests to the relevant ViewModel flows.
- Around line 892-901: Remove the redundant
withContext(kotlinx.coroutines.Dispatchers.IO) wrapper around the
syncSubscriptions call in the PodcastInfoViewModel flow, keeping the sync and
updateLatestEpisode logic unchanged. Drop the related withContext/Dispatchers.IO
import if it is no longer used elsewhere.
- Around line 347-353: Guard both libraryTip call sites in PodcastInfoViewModel
so updateLatestEpisode runs only when the podcast is currently subscribed, while
preserving the existing tip and episode arguments. Add a JVM regression test
covering a direct-feed refresh with an unsubscribed podcast and verifying no
latest-episode state is persisted.
In
`@feature/info/src/test/java/cx/aswin/boxlore/feature/info/logic/EpisodeSupplementMergeLogicTest.kt`:
- Around line 46-61: Update the unionSearchResults test to assert the surviving
shared entry’s title, locking the intended network-versus-supplement precedence
when duplicate IDs are merged. Add empty-input tests covering both merge and
unionSearchResults, including their expected empty results, while preserving the
existing ordering and deduplication assertions.
In `@scripts/check-new-episodes-lib.js`:
- Around line 176-185: Update the PI notification path around newest.piEpisodeId
to persist a cross-source episode identity, such as newest.enclosureUrl or guid,
in nextState alongside the existing RSS state. Update the RSS comparison logic
to treat a matching stored cross-source identity as unchanged, then clear or
replace that marker when processing a distinct episode so each branch suppresses
duplicate notifications.
- Around line 25-43: Update durationMinutes so the colon-separated parsing
branch rejects negative computed durations before converting seconds to minutes,
matching the non-colon branch’s '0' behavior. Preserve existing handling for
valid colon-formatted values and use the existing duration validation flow
around seconds.
In `@scripts/check-new-episodes-lib.test.js`:
- Around line 147-183: Add a direct durationMinutes test covering empty/null
input, seconds, mm:ss, hh:mm:ss, malformed values, and negative values. Include
the negative colon-form case (such as “-1:00”) to regress the clamping fix,
asserting all invalid or negative inputs return “0” while valid inputs normalize
correctly.
In `@scripts/check-new-episodes.js`:
- Around line 124-127: Update the trackedPodcasts processing loop to handle
shows in bounded-concurrency batches, respecting Podcast Index rate limits,
instead of issuing network requests sequentially. Add a wall-clock budget for
the run and, when it expires, persist the current state before stopping so
partial progress survives.
- Around line 63-72: Update fetchPiLatest to use an AbortController with the
same timeout behavior as fetchText, passing its signal to fetch so hung Podcast
Index requests terminate predictably. Encode podcastId with the appropriate
URL-component encoding before interpolating it into the request URL.
- Around line 178-187: Track notification delivery with a delivered flag in both
the RSS path at scripts/check-new-episodes.js lines 178-187 and the Podcast
Index path at lines 207-223. Keep it true when decision.notify is false, set it
false when sendFcm fails, and require decision.reason !== 'unchanged' &&
delivered before persisting decision.nextState in both branches so failed
notifications are retried.
- Around line 38-61: Update fetchText to enforce maxBytes before fully buffering
the response: reject a numeric Content-Length exceeding maxBytes immediately,
then consume response.body via async iteration while tracking received bytes and
aborting or throwing once the limit is exceeded. Preserve the existing timeout
cleanup, HTTP status handling, and UTF-8 decoding behavior.
---
Outside diff comments:
In
`@feature/home/src/main/java/cx/aswin/boxlore/feature/home/HomeFeedYourShows.kt`:
- Around line 29-47: Freeze the skeleton layout mode once per loading session
instead of updating skeletonLayoutCount from later Room emissions: in
HomeFeedYourShows.kt, initialize an immutable one-row or two-row mode when
loading begins and pass it through YourShowsFeedContent. In HomeSkeleton.kt
lines 224-227, accept that fixed mode and stop recalculating the row count from
subscribedCount; update both affected call paths consistently.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 4012b5e8-f995-4221-8029-b586a5e87bfb
📒 Files selected for processing (77)
.github/workflows/new-episode-check.ymlARCHITECTURE.mdREADME.mdapp/README.mdapp/src/main/java/cx/aswin/boxlore/AppContainer.ktapp/src/main/java/cx/aswin/boxlore/fcm/BoxLoreFcmService.ktapp/src/main/java/cx/aswin/boxlore/fcm/FcmPayloadParser.ktapp/src/main/java/cx/aswin/boxlore/fcm/NewEpisodeFcmLogic.ktapp/src/main/java/cx/aswin/boxlore/fcm/NewEpisodePushHydration.ktapp/src/main/java/cx/aswin/boxlore/navigation/NavGraphPodcastEpisodeDestinations.ktapp/src/test/java/cx/aswin/boxlore/fcm/FcmPayloadParserTest.ktapp/src/test/java/cx/aswin/boxlore/fcm/NewEpisodeFcmLogicTest.ktapp/src/test/java/cx/aswin/boxlore/fcm/NewEpisodePushHydrationTest.ktcore/catalog/README.mdcore/catalog/src/main/java/cx/aswin/boxlore/core/catalog/LatestEpisodeTipLogic.ktcore/catalog/src/main/java/cx/aswin/boxlore/core/catalog/PodcastEpisodeSupplementMerge.ktcore/catalog/src/main/java/cx/aswin/boxlore/core/catalog/PodcastRepository.ktcore/catalog/src/main/java/cx/aswin/boxlore/core/catalog/PodcastRepositoryEpisodeSupplements.ktcore/catalog/src/main/java/cx/aswin/boxlore/core/catalog/PodcastRepositoryNetworkLookups.ktcore/catalog/src/main/java/cx/aswin/boxlore/core/catalog/SubscriptionForegroundSync.ktcore/catalog/src/main/java/cx/aswin/boxlore/core/catalog/SubscriptionRepository.ktcore/catalog/src/main/java/cx/aswin/boxlore/core/catalog/TrackedPodcastRtdbLogic.ktcore/catalog/src/test/java/cx/aswin/boxlore/core/catalog/LatestEpisodeTipLogicTest.ktcore/catalog/src/test/java/cx/aswin/boxlore/core/catalog/PodcastEpisodeSupplementMergeTest.ktcore/catalog/src/test/java/cx/aswin/boxlore/core/catalog/PodcastRepositoryEpisodeSupplementTest.ktcore/catalog/src/test/java/cx/aswin/boxlore/core/catalog/SubscriptionForegroundSyncTest.ktcore/catalog/src/test/java/cx/aswin/boxlore/core/catalog/SubscriptionRepositoryTest.ktcore/catalog/src/test/java/cx/aswin/boxlore/core/catalog/TrackedPodcastRtdbLogicTest.ktcore/database/README.mdcore/database/schemas/cx.aswin.boxlore.core.database.BoxLoreDatabase/31.jsoncore/database/src/main/java/cx/aswin/boxlore/core/database/BoxLoreDatabase.ktcore/database/src/main/java/cx/aswin/boxlore/core/database/BoxLoreDatabaseMigrations.ktcore/database/src/main/java/cx/aswin/boxlore/core/database/EpisodeSupplementDao.ktcore/database/src/main/java/cx/aswin/boxlore/core/database/EpisodeSupplementEntity.ktcore/database/src/main/java/cx/aswin/boxlore/core/database/EpisodeSupplementItemEntity.ktcore/database/src/main/java/cx/aswin/boxlore/core/database/PodcastDao.ktcore/database/src/test/java/cx/aswin/boxlore/core/database/PodcastDaoExtendedTest.ktcore/domain/README.mdcore/domain/src/main/java/cx/aswin/boxlore/core/domain/ports/EpisodeSupplementPort.ktcore/model/src/main/java/cx/aswin/boxlore/core/model/Podcast.ktcore/model/src/test/java/cx/aswin/boxlore/core/model/PodcastLatestEpisodeNewTest.ktcore/playback/README.mdcore/playback/src/main/java/cx/aswin/boxlore/core/playback/SmartQueueSources.ktcore/playback/src/test/java/cx/aswin/boxlore/core/playback/SmartQueueEngineTest.ktcore/rss/README.mdcore/rss/src/main/java/cx/aswin/boxlore/core/rss/EpisodeSupplementDisconnectLogic.ktcore/rss/src/main/java/cx/aswin/boxlore/core/rss/EpisodeSupplementListMerge.ktcore/rss/src/main/java/cx/aswin/boxlore/core/rss/EpisodeSupplementMatcher.ktcore/rss/src/main/java/cx/aswin/boxlore/core/rss/EpisodeSupplementRepository.ktcore/rss/src/main/java/cx/aswin/boxlore/core/rss/EpisodeSupplementTipLogic.ktcore/rss/src/test/java/cx/aswin/boxlore/core/rss/EpisodeSupplementDisconnectLogicTest.ktcore/rss/src/test/java/cx/aswin/boxlore/core/rss/EpisodeSupplementMatcherTest.ktcore/rss/src/test/java/cx/aswin/boxlore/core/rss/EpisodeSupplementTipLogicTest.ktfeature/home/README.mdfeature/home/src/main/java/cx/aswin/boxlore/feature/home/HomeFeedYourShows.ktfeature/home/src/main/java/cx/aswin/boxlore/feature/home/components/HeroGridCard.ktfeature/home/src/main/java/cx/aswin/boxlore/feature/home/components/HomeSkeleton.ktfeature/home/src/main/java/cx/aswin/boxlore/feature/home/components/LibrarySectionRows.ktfeature/info/README.mdfeature/info/src/main/java/cx/aswin/boxlore/feature/info/InfoViewModelAssembler.ktfeature/info/src/main/java/cx/aswin/boxlore/feature/info/PodcastInfoScreen.ktfeature/info/src/main/java/cx/aswin/boxlore/feature/info/PodcastInfoSupplementSupport.ktfeature/info/src/main/java/cx/aswin/boxlore/feature/info/PodcastInfoUiModels.ktfeature/info/src/main/java/cx/aswin/boxlore/feature/info/PodcastInfoViewModel.ktfeature/info/src/main/java/cx/aswin/boxlore/feature/info/components/PodcastInfoChrome.ktfeature/info/src/main/java/cx/aswin/boxlore/feature/info/logic/EpisodeSupplementEligibility.ktfeature/info/src/main/java/cx/aswin/boxlore/feature/info/logic/EpisodeSupplementMergeLogic.ktfeature/info/src/test/java/cx/aswin/boxlore/feature/info/logic/EpisodeSupplementEligibilityTest.ktfeature/info/src/test/java/cx/aswin/boxlore/feature/info/logic/EpisodeSupplementMergeLogicTest.ktfeature/library/README.mdfeature/library/src/main/java/cx/aswin/boxlore/feature/library/subscriptions/SubscriptionRows.ktscripts/README.mdscripts/check-new-episodes-lib.jsscripts/check-new-episodes-lib.test.jsscripts/check-new-episodes.jsscripts/data/episode-tracker.jsonscripts/package.json
💤 Files with no reviewable changes (1)
- scripts/data/episode-tracker.json
Keep FCM hydration on the notified feed item, hide raw feed errors, and split oversized APIs so detekt, Sonar, and the repository test guard pass.
CI has no BOXLORE_API_BASE_URL, so loading BoxLoreApplication crashed OkHttp before the new-episode helpers ran.
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
scripts/check-new-episodes-lib.js (1)
25-39: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winReject invalid clock components.
At Line 29,
Number(p) || 0converts invalid and negative components to0.durationMinutes("1:bad")returns"1".durationMinutes("1:-30")also returns"1".Reject a clock value when any component is empty, non-finite, or negative. Add regression tests in
scripts/check-new-episodes-lib.test.js.As per coding guidelines, “every bug fix must add a regression test for the same shared failure mode where applicable”.
Proposed fix
- const parts = s.split(':').map((p) => Number(p) || 0); + const rawParts = s.split(':'); + if ( + ![2, 3].includes(rawParts.length) || + rawParts.some((part) => part.trim() === '') + ) { + return '0'; + } + const parts = rawParts.map(Number); + if (parts.some((part) => !Number.isFinite(part) || part < 0)) { + return '0'; + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/check-new-episodes-lib.js` around lines 25 - 39, Update durationMinutes to validate each colon-separated clock component before calculating seconds: reject and return "0" when any component is empty, non-finite, or negative instead of coercing it with Number(p) || 0. Add regression coverage in check-new-episodes-lib.test.js for invalid and negative components, including "1:bad" and "1:-30".Source: Coding guidelines
core/catalog/src/main/java/cx/aswin/boxlore/core/catalog/SubscriptionRepository.kt (1)
341-350: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winThe
shouldReplaceguard can dropmarkAsNewfor an already-stored tip.The early return at Line 344 runs before the
markHasNewEpisodescall at Line 349. When the storedlatestEpisodealready equals the incoming tip, the method returns and never setsrssHasNewEpisodes.This path is reachable.
SubscriptionForegroundSync.createwiressaveLatest = { id, episode -> subscriptionRepository.updateLatestEpisode(id, episode) }, which uses the defaultmarkAsNew = false. If that sync stores the tip first, the laterNewEpisodePushHydration.resolveLocalEpisodecall withmarkAsNew = trueis dropped, and the Home NEW badge never appears for that episode.Apply the new-episode flag before the replace decision.
🛠️ Proposed fix
if (enrichedEpisode != null) { val existing = podcastDao.getPodcast(podcastId)?.latestEpisode if (!LatestEpisodeTipLogic.shouldReplace(existing, enrichedEpisode)) { + if (markAsNew) { + podcastDao.markHasNewEpisodes(podcastId) + } return } }Add a JVM regression test in
core/catalog/src/test/java/cx/aswin/boxlore/core/catalog/SubscriptionRepositoryTest.ktthat stores a tip withmarkAsNew = falseand then re-promotes the same tip withmarkAsNew = true.As per coding guidelines: "Extend JVM tests under
src/testfor touched logic; every bug fix must add a regression test for the same shared failure mode where applicable."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@core/catalog/src/main/java/cx/aswin/boxlore/core/catalog/SubscriptionRepository.kt` around lines 341 - 350, Update updateLatestEpisode so markHasNewEpisodes is applied for markAsNew = true before the LatestEpisodeTipLogic.shouldReplace early return, while preserving replacement behavior for genuinely newer tips. Add a JVM regression test in SubscriptionRepositoryTest that stores a tip with markAsNew = false, then re-promotes the same tip with markAsNew = true and verifies the new-episode flag is set.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/src/test/java/cx/aswin/boxlore/fcm/NewEpisodePushHydrationTest.kt`:
- Around line 156-188: Update
doesNotPromoteUnrelatedNewestWhenPayloadEnclosureDiffers to capture the FakePort
request or its request.match and assert that the propagated GUID equals
payloadGuid ("guid-ep"). Add a GUID-only regression case using the same
resolveLocalEpisode path, if supported by the existing test helpers, so the test
fails when payloadGuid is dropped rather than relying only on enclosure
matching.
In
`@core/catalog/src/main/java/cx/aswin/boxlore/core/catalog/PodcastRepositoryNetworkLookups.kt`:
- Around line 68-72: Update PodcastRepository.getEpisodeImpl and its
navigation/playback callers to accept an owner-scoped episode reference, then
query only the backing store identified by that owner instead of probing both
stores for negative IDs. Preserve distinct resolution for colliding PI
supplement and rss episodes, and add a regression test covering both owners
resolving to their respective episodes.
In `@core/rss/README.md`:
- Line 16: Update the README bullet describing merge ownership: limit the “only”
PodcastRepository claim to episode-list page merges, and explicitly identify
EpisodeSupplementMergeLogic.unionSearchResults as the separate in-show search
merge site. Preserve the existing identity-rule and preferred-list details.
In
`@core/rss/src/main/java/cx/aswin/boxlore/core/rss/EpisodeSupplementRepository.kt`:
- Around line 137-141: Update the URL resolution in the relevant
EpisodeSupplementRepository flow to use the trimmed request URL only when it
starts with HTTPS; otherwise fall back to the stored existing.feedUrl and
validate that result before continuing. Add a JVM regression test in
EpisodeSupplementRepositoryTest covering a non-HTTPS request URL paired with an
HTTPS stored URL, asserting the stored HTTPS URL is used successfully.
In
`@core/rss/src/test/java/cx/aswin/boxlore/core/rss/EpisodeSupplementListMergeTest.kt`:
- Around line 20-44: Extend EpisodeSupplementListMergeTest with a case where a
supplement sharing an audio URL with a Podcast Index episode is deduplicated,
asserting the matching supplement is removed. Add coverage for
EpisodeSupplementListMerge.Sort.OLDEST and assert the merged episodes are
ordered oldest first, while preserving the existing distinct same-title/newest
case.
In
`@core/rss/src/test/java/cx/aswin/boxlore/core/rss/EpisodeSupplementMatcherTest.kt`:
- Around line 137-157: Add JVM tests that directly exercise
EpisodeSupplementMatcher.isDuplicateOf, covering matching IDs, matching audio
URLs, both sides having blank audio URLs, and identical titles with dates far
apart. Keep the assertions focused on the identity result and place the cases
alongside the existing matcher tests.
In
`@feature/info/src/main/java/cx/aswin/boxlore/feature/info/PodcastInfoScreen.kt`:
- Around line 721-733: Reuse the existing jumpPillVisible value for the
jump-pill visibility check around the pill declaration instead of repeating
targetJumpEpisode != null && !isTargetVisible && isFabVisible. Also replace the
snackbar offset’s 56.dp literal with the existing pill-height symbol declared
near the jump pill, keeping both layout calculations synchronized.
In
`@feature/info/src/main/java/cx/aswin/boxlore/feature/info/PodcastInfoSupplementSupport.kt`:
- Around line 218-221: The in-show search merge in the surrounding function must
prefer network results, matching PodcastEpisodeSupplementMerge and the
duplicate-identity behavior of EpisodeSupplementListMerge.unionSearchResults.
Swap the arguments passed to unionSearchResults so networkResults is preferred
and supplementMatches is the fallback.
In
`@feature/info/src/test/java/cx/aswin/boxlore/feature/info/PodcastInfoSupplementSupportTest.kt`:
- Around line 15-49: Add a hermetic JVM test alongside the existing
refreshMissingEpisodes success test, configuring FakePort.refreshOutcome as
EpisodeSupplementOutcome.Failure and invoking
PodcastInfoSupplementSupport.refreshMissingEpisodes. Assert that
refresh.state.userMessage equals the failure outcome message and
refresh.libraryTip is null, while preserving the existing page reload setup.
- Around line 149-217: Move the configurable EpisodeSupplementPort fake
represented by FakePort into :core:testing, then replace the equivalent local
fakes in PodcastInfoSupplementSupportTest,
PodcastRepositoryEpisodeSupplementTest, and NewEpisodePushHydrationTest with the
shared fake. Preserve configurable outcomes, opted-in IDs, and search results,
and update test dependencies/imports so all three tests consume the single
implementation.
---
Outside diff comments:
In
`@core/catalog/src/main/java/cx/aswin/boxlore/core/catalog/SubscriptionRepository.kt`:
- Around line 341-350: Update updateLatestEpisode so markHasNewEpisodes is
applied for markAsNew = true before the LatestEpisodeTipLogic.shouldReplace
early return, while preserving replacement behavior for genuinely newer tips.
Add a JVM regression test in SubscriptionRepositoryTest that stores a tip with
markAsNew = false, then re-promotes the same tip with markAsNew = true and
verifies the new-episode flag is set.
In `@scripts/check-new-episodes-lib.js`:
- Around line 25-39: Update durationMinutes to validate each colon-separated
clock component before calculating seconds: reject and return "0" when any
component is empty, non-finite, or negative instead of coercing it with
Number(p) || 0. Add regression coverage in check-new-episodes-lib.test.js for
invalid and negative components, including "1:bad" and "1:-30".
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: d398e7dc-d0a1-48cf-a43b-4d7792cf0488
📒 Files selected for processing (44)
.github/workflows/new-episode-check.ymlapp/src/main/java/cx/aswin/boxlore/fcm/BoxLoreFcmService.ktapp/src/main/java/cx/aswin/boxlore/fcm/NewEpisodePushHydration.ktapp/src/test/java/cx/aswin/boxlore/fcm/NewEpisodeFcmLogicTest.ktapp/src/test/java/cx/aswin/boxlore/fcm/NewEpisodePushHydrationTest.ktcore/catalog/README.mdcore/catalog/src/main/java/cx/aswin/boxlore/core/catalog/PodcastEpisodeSupplementMerge.ktcore/catalog/src/main/java/cx/aswin/boxlore/core/catalog/PodcastRepositoryNetworkLookups.ktcore/catalog/src/main/java/cx/aswin/boxlore/core/catalog/SubscriptionForegroundSync.ktcore/catalog/src/main/java/cx/aswin/boxlore/core/catalog/SubscriptionRepository.ktcore/catalog/src/test/java/cx/aswin/boxlore/core/catalog/PodcastRepositoryEpisodeSupplementTest.ktcore/catalog/src/test/java/cx/aswin/boxlore/core/catalog/SubscriptionForegroundSyncTest.ktcore/database/README.mdcore/database/src/main/java/cx/aswin/boxlore/core/database/EpisodeSupplementDao.ktcore/domain/README.mdcore/domain/src/main/java/cx/aswin/boxlore/core/domain/ports/EpisodeSupplementPort.ktcore/model/README.mdcore/playback/src/test/java/cx/aswin/boxlore/core/playback/SmartQueueEngineSupplementTest.ktcore/playback/src/test/java/cx/aswin/boxlore/core/playback/SmartQueueEngineTest.ktcore/rss/README.mdcore/rss/src/main/java/cx/aswin/boxlore/core/rss/EpisodeSupplementListMerge.ktcore/rss/src/main/java/cx/aswin/boxlore/core/rss/EpisodeSupplementMatcher.ktcore/rss/src/main/java/cx/aswin/boxlore/core/rss/EpisodeSupplementRepository.ktcore/rss/src/main/java/cx/aswin/boxlore/core/rss/EpisodeSupplementTipLogic.ktcore/rss/src/test/java/cx/aswin/boxlore/core/rss/EpisodeSupplementDisconnectLogicTest.ktcore/rss/src/test/java/cx/aswin/boxlore/core/rss/EpisodeSupplementListMergeTest.ktcore/rss/src/test/java/cx/aswin/boxlore/core/rss/EpisodeSupplementMatcherTest.ktcore/rss/src/test/java/cx/aswin/boxlore/core/rss/EpisodeSupplementRepositoryTest.ktcore/rss/src/test/java/cx/aswin/boxlore/core/rss/EpisodeSupplementTipLogicTest.ktfeature/info/README.mdfeature/info/src/main/java/cx/aswin/boxlore/feature/info/InfoViewModelAssembler.ktfeature/info/src/main/java/cx/aswin/boxlore/feature/info/PodcastInfoScreen.ktfeature/info/src/main/java/cx/aswin/boxlore/feature/info/PodcastInfoSupplementSupport.ktfeature/info/src/main/java/cx/aswin/boxlore/feature/info/PodcastInfoUiModels.ktfeature/info/src/main/java/cx/aswin/boxlore/feature/info/PodcastInfoViewModel.ktfeature/info/src/main/java/cx/aswin/boxlore/feature/info/components/PodcastInfoChrome.ktfeature/info/src/main/java/cx/aswin/boxlore/feature/info/logic/EpisodeSupplementMergeLogic.ktfeature/info/src/test/java/cx/aswin/boxlore/feature/info/PodcastInfoSupplementSupportTest.ktfeature/info/src/test/java/cx/aswin/boxlore/feature/info/logic/EpisodeSupplementMergeLogicTest.ktscripts/README.mdscripts/check-new-episodes-lib.jsscripts/check-new-episodes-lib.test.jsscripts/check-new-episodes.jsscripts/data/episode-tracker.json
Prefer a stored HTTPS feed URL when Room has http, assert FCM guid propagation, and add merge/matcher/refresh failure coverage.
|
ashwkun
added a commit
that referenced
this pull request
Aug 12, 2026
Upcoming / What's New copy for #971 now names missing episodes so listeners do not read it as the show being stale.
14 tasks
This was referenced Aug 15, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Podcast Index (PI) is often a drop behind a show’s public RSS/Atom feed. Listeners then saw a stale episode list, a stale Home “NEW” tip, and (if they had show notifications on) a push that waited for PI instead of the publisher.
This PR does three things:
rss:library row. Publisher-feed extras are an episode supplement under the stable PI id.PodcastRepository— Podcast Info, Home Your Shows NEW, the New episodes chip, in-show search, Smart Queue.feedUrl; the phone hydrates the local episode (including feed-only ids) before notify / auto-download.Master already pastes filled Release copy regions into CHANGELOG and README What’s New as written (no Groq rewrite). This PR is labeled
user-impact-criticaland fills those regions below.Motivation
Catalog ownership is Podcast Index. That is still true. The bug is lag, not identity.
A concrete live example while this was built: Conspiracy Theories (PI
9926) still showed PI’s 5 Aug 2026 episode (“Was 'The Battle of the Sexes' Fixed?”) while the publisher feed already had 12 Aug 2026 (“UFO Over the Bass Triangle? The Disappearance of Fred Valentich”). Listeners who follow that show weekly were missing a drop in the list, on Home, and in notifications until Index caught up.Settings Add RSS is the wrong fix: it would mint
rss:rows, duplicate the show in the library, and break the PI id that subscriptions, FCM topics (new_ep_{id}), and deep links already use.What listeners see
feedUrl, boxlore can compare PI vs the publisher feed.feedUrlto RTDB. The checker can then fire when the feed updates, even if PI is still behind.What changed (engineering)
Episode supplement (not
rss:)EpisodeSupplementMatcher/ Roomepisode_supplements).rss:ids) are unchanged and never go through this merge.PodcastRepositoryis the single mergegetEpisodes, offset-0getEpisodesPaginated, and in-showsearchEpisodesunion cached feed extras after the PI page.rss:and a null supplement port.EpisodePage.sourceCountstays the PI count before extras (pagination math unchanged).Subscribe / Info / sync
syncSubscriptionsomits opted-in ids fromPOST /syncso the proxy does not overwrite a fresher local list.SubscriptionForegroundSync+ Podcast Info.LatestEpisodeTipLogic: never write an older tip over a newer one; same date + different id still replaces (PI catch-up). Feed promotes useupdateLatestEpisode(..., markAsNew = true)so Home NEW / hero chip light up.Notifications + RTDB
setNotificationsEnabled(true)writestracked_podcasts/{podcastIndexId}={ title, imageUrl }and, if opted in, HTTPSfeedUrl. FCM topicnew_ep_{id}.syncTrackedPodcastFeedUrlafter Info opt-in if notifications are already on. Restore reconcile + process-onceSubscriptionForegroundSyncalso patchfeedUrlfor opted-in notifying shows.scripts/data/episode-tracker.json(that file is the Action’s cursor).Check New Episodes (GHA, ~30 min — not VPS catalog sync)
feedUrl, poll RSS/Atom. Change key = guid, else enclosure (lastRssKey). First see = baseline, no notify.episodes/byfeedid?max=1vslastEpisodeId(old path).lastRssKey.new_ep_{piId}. PIepisodeIdonly if the feed item matches PI (enclosure/guid). Never mint negative ids in GHA. Unmatched → omitepisodeId, routeboxlore://podcast/{id}. Extra FCM keys:feedUrl,guid,enclosureUrl(older clients ignore them).Phone FCM
NewEpisodePushHydrationrefreshes the feed (resolveNewestTipFromFeed), promotes the Room tipmarkAsNew, then notify + autodownload with the local id (PI or negative supplement id).Behavior & compatibility
episodeIdwhen present.rss:library rows are unchanged.rss:ids, Room names, FCM topic shape, andapplicationIdare unchanged.Impact (required)
User impact — pick exactly one
user-impact-criticaluser-impact-highuser-impact-mediumuser-impact-lowno-user-impactListener impact — required when
user-impact-critical,user-impact-high, oruser-impact-mediumWhat changes in the user’s life:
Some shows were lagging behind in the Podcast Index catalog. boxlore now checks freshness when you subscribe and keeps that show updated from Podcast Index or the publisher’s feed, whichever is current. If a show still looks behind — or you haven’t subscribed yet — Missing episodes? on the show page forces the publisher feed. Show notifications follow the same path.
Backend — optional, pairable with any user-impact level
backend-changeRelease copy (verbatim — highest priority)
CHANGELOG.md (developer copy)
Added
Changed
Fixed
README What's New / Upcoming (listener copy)
Critical
Test plan
./gradlew installDebug) when UI or app behavior changedTrackedPodcastRtdbLogic,NewEpisodeFcmLogic,NewEpisodePushHydration,LatestEpisodeTipLogic,node --test scripts/check-new-episodes-lib.test.js)feedUrltracked_podcasts/1258562includesfeedUrlNotes (optional)
feedUrlbackfilled on 60 tracked shows (one-time ops, not in git). Four rows still lack HTTPSfeedUrl(PodQuiz, Percy Jackson HTTP 451, Watchman Privacy / retitled feed, Crimes Reais Revelados).1258562) was deleted from live RTDB and removed fromscripts/data/episode-tracker.jsonso a clean re-subscribe can test the clientfeedUrlwrite. Feed:https://anchor.fm/s/f6585ee8/podcast/rss.CHANGELOG.mdor README Upcoming / What’s New in this PR — the Release copy regions above are the source.changelog-on-mergeon master already pastes filled regions verbatim.