feat(feeds): refresh shorts and livestreams in FEED_REFRESH, with per…#891
feat(feeds): refresh shorts and livestreams in FEED_REFRESH, with per…#891t6fb3m59 wants to merge 2 commits into
Conversation
|
Warning Review limit reached
More reviews will be available in 37 minutes. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?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 credits. 🚦 How do rate 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 see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughThe feed-refresh loop now uses three new flags to control videos, shorts, and livestream tab refreshes. ChannelHelpers now resolves tab ids through a shared helper and refreshes non-null tabs asynchronously before updating channel videos. ChangesConfigurable channel feed refresh
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/main/java/me/kavin/piped/Main.java (2)
284-292: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winIndependent tabs share one failure path.
The three tab refreshes are conceptually independent, but they're wrapped in a single per-channel
try(Lines 293-295). IfvideosTabInfo(info)throws (e.g.ExtractionException/IOException), shorts and livestreams are skipped for that channel even though they might have succeeded. Consider isolating each enabled tab so one tab's failure doesn't suppress the others.♻️ Isolate per-tab failures
if (Constants.FEED_REFRESH_VIDEOS) { try { ChannelHelpers.refreshChannelTab(info, ChannelHelpers.videosTabInfo(info)); } catch (Exception e) { ExceptionHandler.handle(e); } } // repeat for shorts / livestreams🤖 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 `@src/main/java/me/kavin/piped/Main.java` around lines 284 - 292, The three refreshes in Main’s per-channel flow are independent but currently share one try path, so a failure in ChannelHelpers.videosTabInfo/info refresh can prevent shorts and livestreams from running. Update the Main channel refresh logic to handle each enabled tab separately by wrapping each ChannelHelpers.refreshChannelTab call (videosTabInfo, shortsTabInfo, livestreamsTabInfo) in its own try/catch so one tab’s exception does not skip the others.
274-277: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low value
delay/period no longer reflects the real refresh cycle.
delayis derived fromchannelIds.size()andFEED_REFRESH_MINUTES, but each channel now performs up to three sequentialChannelTabInfo.getInfo(...)network fetches instead of one. The effective time to traverse all channels can be ~3x the configured period, so the cycle silently overrunsFEED_REFRESH_MINUTES. Consider accounting for the number of enabled tabs in the pacing, or at least documenting that the period is best-effort.🤖 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 `@src/main/java/me/kavin/piped/Main.java` around lines 274 - 277, The FeedRefresh pacing in Main should be updated because the current delay calculation based on channelIds.size() and FEED_REFRESH_MINUTES no longer matches the real refresh cycle now that each channel can trigger up to three ChannelTabInfo.getInfo calls. Adjust the scheduling logic around the delay/period calculation to account for the extra per-channel fetch work, or explicitly document in the FeedRefresh logging/behavior that the configured period is only best-effort and may overrun.
🤖 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.
Nitpick comments:
In `@src/main/java/me/kavin/piped/Main.java`:
- Around line 284-292: The three refreshes in Main’s per-channel flow are
independent but currently share one try path, so a failure in
ChannelHelpers.videosTabInfo/info refresh can prevent shorts and livestreams
from running. Update the Main channel refresh logic to handle each enabled tab
separately by wrapping each ChannelHelpers.refreshChannelTab call
(videosTabInfo, shortsTabInfo, livestreamsTabInfo) in its own try/catch so one
tab’s exception does not skip the others.
- Around line 274-277: The FeedRefresh pacing in Main should be updated because
the current delay calculation based on channelIds.size() and
FEED_REFRESH_MINUTES no longer matches the real refresh cycle now that each
channel can trigger up to three ChannelTabInfo.getInfo calls. Adjust the
scheduling logic around the delay/period calculation to account for the extra
per-channel fetch work, or explicitly document in the FeedRefresh
logging/behavior that the configured period is only best-effort and may overrun.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2ffc08de-94a5-40cc-8979-e04261f24981
📒 Files selected for processing (3)
src/main/java/me/kavin/piped/Main.javasrc/main/java/me/kavin/piped/consts/Constants.javasrc/main/java/me/kavin/piped/utils/ChannelHelpers.java
t6fb3m59
force-pushed
the
upstream-pr-feed-refresh-types
branch
from
1b3ac98 to
05370a9
Compare
…-type toggles The FEED_REFRESH loop currently pulls only the videos tab of each subscribed channel; Shorts and livestream tabs are silently skipped. Surface them too and expose three independent boolean toggles so admins can disable any type to cut the YouTube requests issued per channel per cycle. FEED_REFRESH_VIDEOS default true pulls the videos tab FEED_REFRESH_SHORTS default true pulls the shorts tab FEED_REFRESH_LIVESTREAMS default true pulls the livestreams tab FEED_REFRESH itself remains the master switch; when off, none of the new flags matter. ChannelHelpers.videosTabInfo becomes a thin wrapper around a new private tabInfo(info, tabId) helper which the new shortsTabInfo and livestreamsTabInfo also use, keeping the tab-extraction logic in one place. A new ChannelHelpers.refreshChannelTab(info, tabInfo) consolidates the federate-then-update pattern that the loop runs for each enabled tab.
t6fb3m59
force-pushed
the
upstream-pr-feed-refresh-types
branch
from
05370a9 to
2898707
Compare
…s null VideoHelpers.handleNewVideo wrote Math.max(infoTime, time) to the uploaded column. Both sources independently fall back to System.currentTimeMillis() when their underlying date is null, so Math.max(realDate, NOW) = NOW clobbered any real date whenever one side was missing. NPE returns null upload dates for every Shorts channel-tab listing item, so every short pulled via FEED_REFRESH landed in the DB with a fetch-time timestamp. Because the feed query orders by uploaded DESC, every cycle pushed shorts back to the top of the feed, burying long-form rows below the visible window of any client that paginates or limits its render. The same shape fires for any future tab whose listing returns a null date. Pre-existing upstream bug from 0cda383 (Oct 2023); never triggered before because the Videos tab and PubSub feed both supplied real listing dates, making Math.max a no-op. The recent per-type FEED_REFRESH toggle that routes Shorts through updateChannelVideos exposes it widely. Add VideoHelpers.pickUploaded(DateWrapper, long) returning the larger of two real dates, the single real date when only one is known, and System.currentTimeMillis() only when both are absent. Callers that previously substituted NOW for a missing listing date now pass -1L as a sentinel. The pre-fetch retention gate in updateChannelVideos and saveChannel keeps its existing NOW-fallback so items with unknown listing dates stay eligible for extraction; the actual retention enforcement inside handleNewVideo continues to use the extractor's real date. PubSub callers are untouched (they always pass a real publishedDate). The Matrix SyncRunner now passes -1L instead of NOW because federated stream.info events do not carry an upload date.
1 participant
The FEED_REFRESH loop currently pulls only the videos tab of each subscribed channel; Shorts and livestream tabs are silently skipped. Surface them too and expose three independent boolean toggles so admins can disable any type to cut the YouTube requests issued per channel per cycle.
FEED_REFRESH_VIDEOS default true pulls the videos tab
FEED_REFRESH_SHORTS default true pulls the shorts tab
FEED_REFRESH_LIVESTREAMS default true pulls the livestreams tab
FEED_REFRESH itself remains the master switch; when off, none of the new flags matter.
ChannelHelpers.videosTabInfo becomes a thin wrapper around a new private tabInfo(info, tabId) helper which the new shortsTabInfo and livestreamsTabInfo also use, keeping the tab-extraction logic in one place.
A new ChannelHelpers.refreshChannelTab(info, tabInfo) consolidates the federate-then-update pattern that the loop runs for each enabled tab.
Summary by CodeRabbit