feat: improve feed tracking and gate handling - #91
Conversation
|
Warning Review limit reached
Next review available in: 34 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. 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. 📝 WalkthroughWalkthroughThe PR adds shared gate automation, byte-based download progress, separate gate and download WebUI stages, and SoundCloud feed-position tracking with navigation controls. ChangesDownload and gate flow
SoundCloud feed navigation
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant DownloadGater
participant GatePopup
participant Server
participant WebUI
DownloadGater->>Server: emit handling_gates progress
DownloadGater->>GatePopup: open target and invoke action
GatePopup-->>DownloadGater: expose confirmation state
DownloadGater->>GatePopup: confirm completion and close popup
Server->>WebUI: send gate or download progress over SSE
WebUI-->>WebUI: render gate handling or byte progress
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
D3SOX
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
userscript/sc-gate-dl.user.js (1)
394-409: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueGrant and use
GM_deleteValuefor the checkpoint.The metadata grants
GM_getValueandGM_setValue, but notGM_deleteValue. Add//@grantGM_deleteValue; otherwise the fallback keeps the storednullvalue.🤖 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 `@userscript/sc-gate-dl.user.js` around lines 394 - 409, Update the userscript metadata to grant GM_deleteValue, then modify resetFeedCheckpoints to call GM_deleteValue for FEED_CHECKPOINT_KEY when available instead of storing null with GM_setValue; preserve the localStorage fallback and existing status/navigation updates.Source: Linters/SAST tools
src/downloadgater.ts (1)
268-338: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider extracting the shared popup-confirm pattern.
handleSpotifyStepduplicates the open-popup/wait/close/confirm sequence already implemented inhandleInstagramStep(Line 363-434 in this file). Both methods: capturepagesBefore, click an action button, poll for a new page within a deadline, wait for a confirm button to become enabled, close the popup, then click confirm. The main differences are the button-text regexes, timeouts, and attempt counts.Extract a shared private helper (for example
handleHonorSystemPopupStep(page, options)) that takes the action-button regex, confirm-button regex, popup-wait timeout, and max attempts as parameters. This removes the duplication and keeps future gate-handling fixes (for example, popup detection tweaks) in one place instead of two divergent copies.Note also that
handleInstagramStepcheckshasDownloadButton(page)explicitly inside its loop, whilehandleSpotifySteprelies only ondetectPane(page) !== 'spotify'. SincedetectPanealready treats a visible download button as the'download'pane, this asymmetry is not a functional bug, but a shared helper would naturally unify the two checks.🤖 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/downloadgater.ts` around lines 268 - 338, Extract the duplicated popup-and-confirm flow from handleSpotifyStep and handleInstagramStep into a shared private helper, such as handleHonorSystemPopupStep. Parameterize the action-button pattern, confirmation-button pattern, popup wait timeout, and maximum attempts, while preserving each method’s existing behavior and pane/download checks. Update both step handlers to call the helper so popup detection, waiting, closing, and confirmation remain centralized.
🤖 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 `@src/server.ts`:
- Around line 208-223: The downloading progress logic in the server update flow
must preserve YtDlpDownloader’s supplied percent when extra.downloadBytes or
extra.totalBytes is unavailable. Update the fallback in the stage ===
'downloading' branch so it does not force non-completion updates to 0% or
completion updates to 100%; retain the existing byte-based calculation when
valid byte values are provided, then pass the preserved stagePercent to
jobStore.updateProgress.
In `@userscript/sc-gate-dl.user.js`:
- Around line 527-562: Reduce repeated full-feed scans by caching each card’s
resolved track URL in a WeakMap reused by findFeedCard and other callers. In
userscript/sc-gate-dl.user.js lines 527-562, debounce the MutationObserver
callback with a short timer and clear that timer in finish; in lines 479-492,
retain the last recorded playing URL and return early when the interval observes
no change. Apply the shared cache so repeated scans reuse parsed card URLs.
- Around line 431-444: Update the fallback branch of shouldAdvanceCheckpoint to
compute the indices of card and savedCard from cards, and return false when
either index is -1. Only apply the existing direction-based comparison after
both cards are confirmed present in the feedCards() result.
---
Nitpick comments:
In `@src/downloadgater.ts`:
- Around line 268-338: Extract the duplicated popup-and-confirm flow from
handleSpotifyStep and handleInstagramStep into a shared private helper, such as
handleHonorSystemPopupStep. Parameterize the action-button pattern,
confirmation-button pattern, popup wait timeout, and maximum attempts, while
preserving each method’s existing behavior and pane/download checks. Update both
step handlers to call the helper so popup detection, waiting, closing, and
confirmation remain centralized.
In `@userscript/sc-gate-dl.user.js`:
- Around line 394-409: Update the userscript metadata to grant GM_deleteValue,
then modify resetFeedCheckpoints to call GM_deleteValue for FEED_CHECKPOINT_KEY
when available instead of storing null with GM_setValue; preserve the
localStorage fallback and existing status/navigation updates.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 189af043-7897-4186-aae7-983df8de5554
📒 Files selected for processing (8)
src/directDownload.tssrc/downloadgater.tssrc/droploud.tssrc/gaterush.tssrc/hypeddit.tssrc/server.tsuserscript/sc-gate-dl.user.jswebui/src/components/App.tsx
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/downloadgater.ts (1)
268-372: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winFallback popup matcher can select a stale tab.
At Line 314, the fallback matcher searches all pages for
options.popupUrlPattern, but it does not exclude pages already present inpagesBefore. The primary matcher at Line 308 excludespagesBeforepages. If the primary matcher times out and a leftover tab from an earlier failed attempt still matches the URL pattern, the fallback selects and closes that stale tab instead of correctly detecting that no new popup opened for this attempt.Add the same
pagesBeforeexclusion to the fallback matcher.🐛 Proposed fix to exclude stale pages from the fallback matcher
popup ??= pages.find( (candidate) => candidate !== page && + !pagesBefore.has(candidate) && new RegExp(options.popupUrlPattern, 'i').test(candidate.url()), );🤖 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/downloadgater.ts` around lines 268 - 372, Update the fallback popup search in handleHonorSystemPopupStep to exclude every page already contained in pagesBefore, matching the primary candidate filter. Ensure stale tabs matching options.popupUrlPattern cannot be selected or closed, while newly opened matching popups remain eligible.
🧹 Nitpick comments (4)
src/downloadgater.ts (1)
724-729: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRedundant percent calculation; see consolidated comment.
downloadPercentis computed here fromreceivedBytes/totalBytes, butsrc/server.ts'semitProgressalways recalculates the same ratio from theextra.downloadBytes/extra.totalBytesfields whenevertotalBytes > 0(which is exactly when this branch runs), discarding the value passed here. See the consolidated comment for the shared fix across files.🤖 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/downloadgater.ts` around lines 724 - 729, Remove the redundant downloadPercent calculation in the download progress flow and stop passing the discarded value to emitProgress. Preserve the existing received/total MB display and rely on the centralized ratio calculation in emitProgress using the download byte fields.src/gaterush.ts (1)
575-580: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRedundant percent calculation; see consolidated comment.
downloadPercentis computed here fromreceivedBytes/totalBytes, butsrc/server.ts'semitProgressalways recalculates the same ratio fromextra.downloadBytes/extra.totalByteswhenevertotalBytes > 0, discarding this value. See the consolidated comment for the shared fix.🤖 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/gaterush.ts` around lines 575 - 580, Remove the redundant downloadPercent calculation near emitProgress in the download flow, since emitProgress recalculates the ratio from downloadBytes and totalBytes. Update the call and related data flow to use the consolidated calculation while preserving the existing zero-total behavior and progress reporting.src/hypedditHttp.ts (1)
288-297: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRedundant percent calculation; see consolidated comment.
This call always has
totalBytes > 0(guarded at Line 289), sosrc/server.ts'semitProgressalways recalculates the same ratio fromextra.downloadBytes/extra.totalBytesand discards the value computed here. See the consolidated comment for the shared fix.🤖 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/hypedditHttp.ts` around lines 288 - 297, Update the progress callback invocation in the download flow around the `lastEmit` and `totalBytes > 0` guard to stop calculating and passing the redundant `(receivedBytes / totalBytes) * 100` percentage. Preserve the existing byte values in the callback metadata and rely on `src/server.ts`'s `emitProgress` to derive the percentage from `downloadBytes` and `totalBytes`.src/hypeddit.ts (1)
719-723: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRedundant percent calculation; see consolidated comment.
downloadPercentis computed here fromreceivedBytes/totalBytes, butsrc/server.ts'semitProgressalways recalculates the same ratio fromextra.downloadBytes/extra.totalByteswhenevertotalBytes > 0, discarding this value. See the consolidated comment for the shared fix.🤖 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/hypeddit.ts` around lines 719 - 723, Remove the redundant local downloadPercent calculation in the download progress flow surrounding emitProgress, since src/server.ts's emitProgress recalculates it from downloadBytes and totalBytes. Update the call to rely on the consolidated calculation while preserving the existing downloading message and progress data.
🤖 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.
Outside diff comments:
In `@src/downloadgater.ts`:
- Around line 268-372: Update the fallback popup search in
handleHonorSystemPopupStep to exclude every page already contained in
pagesBefore, matching the primary candidate filter. Ensure stale tabs matching
options.popupUrlPattern cannot be selected or closed, while newly opened
matching popups remain eligible.
---
Nitpick comments:
In `@src/downloadgater.ts`:
- Around line 724-729: Remove the redundant downloadPercent calculation in the
download progress flow and stop passing the discarded value to emitProgress.
Preserve the existing received/total MB display and rely on the centralized
ratio calculation in emitProgress using the download byte fields.
In `@src/gaterush.ts`:
- Around line 575-580: Remove the redundant downloadPercent calculation near
emitProgress in the download flow, since emitProgress recalculates the ratio
from downloadBytes and totalBytes. Update the call and related data flow to use
the consolidated calculation while preserving the existing zero-total behavior
and progress reporting.
In `@src/hypeddit.ts`:
- Around line 719-723: Remove the redundant local downloadPercent calculation in
the download progress flow surrounding emitProgress, since src/server.ts's
emitProgress recalculates it from downloadBytes and totalBytes. Update the call
to rely on the consolidated calculation while preserving the existing
downloading message and progress data.
In `@src/hypedditHttp.ts`:
- Around line 288-297: Update the progress callback invocation in the download
flow around the `lastEmit` and `totalBytes > 0` guard to stop calculating and
passing the redundant `(receivedBytes / totalBytes) * 100` percentage. Preserve
the existing byte values in the callback metadata and rely on `src/server.ts`'s
`emitProgress` to derive the percentage from `downloadBytes` and `totalBytes`.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 669534f7-c5b2-480d-a653-36594aa9e1f7
📒 Files selected for processing (9)
src/directDownload.tssrc/downloadgater.tssrc/droploud.tssrc/gaterush.tssrc/hypeddit.tssrc/hypedditHttp.tssrc/server.tssrc/ytdlp.tsuserscript/sc-gate-dl.user.js
🚧 Files skipped from review as they are similar to previous changes (3)
- src/droploud.ts
- userscript/sc-gate-dl.user.js
- src/directDownload.ts
|
@coderabbitai rate limit |
|
Your plan includes PR reviews subject to rate limits. More reviews will be available in 38 minutes. |
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/downloadgater.ts (2)
357-359: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winClose the gate page when shared handling fails.
Line [357] throws after retries.
downloadAudiocloses the page only afterhandleDownloadsucceeds. This throw bypassespage.close(), so repeated failed gate attempts can leak browser pages.Move page cleanup into a
finallyblock covering the full gate and download flow.🤖 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/downloadgater.ts` around lines 357 - 359, Update the gate/download flow around downloadAudio and handleDownload so page.close() executes in a finally block covering retries, successful handling, and errors thrown after retries. Preserve the existing error propagation while ensuring every gate page is closed when shared handling fails.
304-351: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winRequire the expected popup before confirming the gate.
Line [308] accepts any new non-blank page. This makes the
popupUrlPatterncheck at Line [318] ineffective when a new page exists. If no popup opens, the code can still click the confirmation button at Line [349]. The handler can advance without opening the required Spotify or Instagram page.Filter candidates by
popupUrlPattern, and skip confirmation when no matching popup is found.Proposed fix
- popup = pages.find( - (candidate) => - candidate !== page && - !pagesBefore.has(candidate) && - candidate.url() !== 'about:blank', - ); - popup ??= pages.find( + popup = pages.find( (candidate) => candidate !== page && !pagesBefore.has(candidate) && - new RegExp(options.popupUrlPattern, 'i').test(candidate.url()), + new RegExp(options.popupUrlPattern, 'i').test(candidate.url()), ); ... + if (!popup) { + await timeout(500); + continue; + }🤖 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/downloadgater.ts` around lines 304 - 351, Update the popup detection loop to only assign `popup` to new pages whose URLs match `options.popupUrlPattern`, removing the fallback that accepts any non-blank page. Before the confirmation wait and `page.evaluate` click in the surrounding gate handler, return or otherwise skip confirmation when no matching popup is found, while preserving confirmation only for a detected, open popup.
🤖 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 `@src/downloadgater.ts`:
- Around line 725-730: Update the in-progress emitProgress call in the download
flow to calculate progress as receivedBytes divided by totalBytes, convert it to
a percentage, and clamp the result between 0 and 100 before passing it instead
of the constant 0. Preserve the existing progress message and byte metadata.
---
Outside diff comments:
In `@src/downloadgater.ts`:
- Around line 357-359: Update the gate/download flow around downloadAudio and
handleDownload so page.close() executes in a finally block covering retries,
successful handling, and errors thrown after retries. Preserve the existing
error propagation while ensuring every gate page is closed when shared handling
fails.
- Around line 304-351: Update the popup detection loop to only assign `popup` to
new pages whose URLs match `options.popupUrlPattern`, removing the fallback that
accepts any non-blank page. Before the confirmation wait and `page.evaluate`
click in the surrounding gate handler, return or otherwise skip confirmation
when no matching popup is found, while preserving confirmation only for a
detected, open popup.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 4ad143bf-d53c-4b90-b423-d5333790153b
📒 Files selected for processing (5)
src/downloadgater.tssrc/droploud.tssrc/gaterush.tssrc/hypeddit.tssrc/hypedditHttp.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/hypedditHttp.ts
1 participant
What changed
SoundCloud feed position controls
Artist — Track Titleby matching the exact track URL and excluding uploader linksGM_deleteValue1.10.2Gate and download workflow
DownloadGater Spotify support
Why
Feed tracking previously stored only one direction at a time, and its broad title selector could display the uploader twice. During deep scrolling, both resume buttons also turned into duplicate cancel buttons.
Gate automation was shown as download progress on a whole-job percentage scale, so Download could become active before any transfer and only move through roughly 75–85%. DownloadGater Spotify gates stopped with an unsupported-OAuth error even though the site only requires opening its Spotify link and confirming the honor-system step.
These changes make the saved feed boundaries, UI stages, transfer progress, and DownloadGater behavior match what the user is actually doing.
Validation
bun test— 32 tests passedbunx tsc --noEmit -p webui/tsconfig.jsonbunx biome check src/server.ts src/directDownload.ts src/hypeddit.ts src/droploud.ts src/gaterush.ts src/downloadgater.tsnode --check userscript/sc-gate-dl.user.jsgit diff --checklimoncello-paint-the-town-red-84870: advanced from Spotify to SoundCloud and left no Spotify tab openlintcheck passedSummary by CodeRabbit
New Features
Improvements