Conversation
WalkthroughThis update introduces a range of changes, including dependency upgrades, the removal of the Changes
Sequence Diagram(s)JioSaavn Playback FlowsequenceDiagram
participant User
participant Player
participant JioSaavnModule
participant JioSaavnAPI
participant UI
User->>Player: Play music request
Player->>JioSaavnModule: If JioSaavn enabled, delegate playback
JioSaavnModule->>UI: Update title (fetching)
JioSaavnModule->>JioSaavnAPI: Search for track (title + author)
JioSaavnAPI-->>JioSaavnModule: Return search results
JioSaavnModule->>JioSaavnModule: Find matching track
alt Track found
JioSaavnModule->>Player: Set audio source to selected quality URL
JioSaavnModule->>UI: Update quality and history
else Track not found / error
JioSaavnModule->>UI: Show error, disable JioSaavn
JioSaavnModule->>Player: Fallback to generic playback
end
Proxy and API Status HandlingsequenceDiagram
participant App
participant Store
participant RemoteAPI
App->>RemoteAPI: Fetch instance data
RemoteAPI-->>App: Return instance config (proxy, health, status)
App->>Store: Update store.api.proxy, store.api.status, etc.
App->>UI: Update health indicator based on status
App->>Utils: Use proxyHandler to determine URL based on enforceProxy or status
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Possibly related PRs
Poem
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (4)
package.json (1)
15-17: Double-check Netlify & Node type bumps for CI/CD compatibilityMinor bumps for
@netlify/*and@types/nodeare low-risk, yet:• Netlify edge functions occasionally add new required config keys—verify the production build still deploys without warnings.
•@types/node@24.1.xbumps lib dom globals; if you target an older Node runtime in Netlify, mismatch warnings can appear. Consider adding an"engines"field (e.g."node": ">=18") to lock expectations.src/stylesheets/header.css (1)
65-69: Consider using 'default' instead of 'none' for better UX.Setting
cursor: nonecompletely hides the cursor, which can be confusing for users. Consider usingcursor: defaulton thelielement instead, which provides a clearer visual hierarchy while maintaining the pointer cursor on the clickable anchor.&:has(a) { - cursor: none; + cursor: default; a { cursor: pointer; } }src/modules/localExtraction.ts (2)
27-36: Improve type safety and resolution handling.const videoStreams = streamingData.adaptiveFormats ? streamingData.adaptiveFormats - .filter((f: any) => f.mimeType?.startsWith('video')) - .map((f: any) => ({ + .filter((f: AdaptiveFormat) => f.mimeType?.startsWith('video')) + .map((f: AdaptiveFormat) => ({ url: f.url, quality: f.qualityLabel || f.quality, - resolution: `${f.width || ''}x${f.height || ''}`.replace(/^x|x$/, ''), + resolution: f.width && f.height ? `${f.width}x${f.height}` : '', type: f.mimeType, })) : [];
38-47: Consider adding type definitions for caption tracks.+interface CaptionTrack { + baseUrl: string; + name?: { runs?: Array<{ text: string }> }; + vssId: string; + languageCode: string; + kind: string; + isTranslatable: boolean; +} + const captionTracks = captions?.playerCaptionsTracklistRenderer?.captionTracks - ? captions.playerCaptionsTracklistRenderer.captionTracks.map((track: any) => ({ + ? captions.playerCaptionsTracklistRenderer.captionTracks.map((track: CaptionTrack) => ({ baseUrl: track.baseUrl, name: track.name?.runs?.[0]?.text || 'Unknown', vssId: track.vssId, languageCode: track.languageCode, kind: track.kind, isTranslatable: track.isTranslatable, })) : [];
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
package.json(1 hunks)src/components/ItemsLoader.ts(1 hunks)src/components/StreamItem.ts(2 hunks)src/components/UpdatePrompt.ts(1 hunks)src/index.d.ts(1 hunks)src/lib/libraryUtils.ts(2 hunks)src/lib/player.ts(2 hunks)src/lib/store.ts(2 hunks)src/lib/utils.ts(1 hunks)src/locales/en.json(1 hunks)src/modules/audioErrorHandler.ts(1 hunks)src/modules/fetchList.ts(2 hunks)src/modules/fetchSearchResults.ts(2 hunks)src/modules/getStreamData.ts(1 hunks)src/modules/jioSaavn.ts(1 hunks)src/modules/listUtils.ts(1 hunks)src/modules/localExtraction.ts(1 hunks)src/stylesheets/header.css(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (5)
src/modules/getStreamData.ts (1)
src/lib/store.ts (1)
state(4-49)
src/modules/fetchSearchResults.ts (1)
src/lib/dom.ts (1)
searchFilters(15-15)
src/modules/audioErrorHandler.ts (1)
src/lib/store.ts (1)
state(4-49)
src/modules/jioSaavn.ts (4)
src/lib/dom.ts (4)
title(3-3)author(5-5)audio(19-19)qualityView(42-42)src/lib/store.ts (3)
store(66-160)state(4-49)params(2-2)src/modules/setMetadata.ts (1)
setMetaData(11-68)src/lib/player.ts (1)
player(8-108)
src/lib/player.ts (2)
src/lib/store.ts (2)
state(4-49)store(66-160)src/lib/dom.ts (2)
title(3-3)playButton(17-17)
🔇 Additional comments (28)
package.json (2)
10-10: Confirmhls.js1.6.7 doesn’t introduce subtle playback regressionsPatch releases are usually safe, but
hls.jssometimes tweaks stream-handling heuristics that can surface edge-case buffering or DRM issues. Please run the existing player e2e tests (especially Safari/iOS) and glance at their changelog for 1.6.7 to ensure nothing breaks silently.
22-23: Build failure: missing TypeScript compilerThe compatibility check (
npm run build && npm run preview) aborted withsh: 1: tsc: not found. Please ensure you have TypeScript installed (e.g. addtypescriptto your devDependencies and runnpm install), or alternatively run the Vite‐only commands:npx vite build && npx vite previewThen confirm no deprecation warnings from
vite-plugin-pwa.src/components/UpdatePrompt.ts (1)
7-8: v7x8 branch verification successful
- Branch “v7x8” exists in n-ce/ytify
- Latest commit: “improve jioSaavn track detection” by n-ce on 2025-07-29T12:25:52Z
- GitHub commits page (https://github.com/n-ce/ytify/commits/v7x8) returns HTTP 200
No further changes required.
src/lib/store.ts (2)
77-78: LGTM! Clean integration of JioSaavn state management.The addition of
useSaavnto the player store object provides a centralized way to track JioSaavn playback preference, properly initialized from the existingstate.jiosaavnconfiguration.
118-118: Good initialization pattern.Properly initializes the
useSaavnflag from the existing state configuration, maintaining consistency with user preferences.src/locales/en.json (1)
117-117: LGTM! Reflects JioSaavn feature stabilization.Removing the "(Beta)" suffix indicates the JioSaavn integration has matured from beta to stable status, which aligns with the codebase refactoring mentioned in the PR.
src/modules/listUtils.ts (1)
53-54: No naming inconsistency detected—conversion from snake_case to camelCase is consistentAll incoming
*_last_updatedfields are uniformly mapped to internallastUpdatedproperties, and the codebase consistently uses camelCase for its TypeScript definitions and assignments. No further changes needed.• src/index.d.ts: defines
lastUpdated?: string
• src/modules/listUtils.ts: mapssender.last_updated→'lastUpdated'
• src/lib/utils.ts & src/lib/libraryUtils.ts: apply the same snake_case → camelCase mapping
• src/components/StreamItem.ts: usesdata.lastUpdatedinternallysrc/components/StreamItem.ts (2)
16-17: LGTM! Clean addition of optional lastUpdated property.The optional
lastUpdatedproperty is properly typed and maintains backward compatibility.
64-64: Good fallback implementation for lastUpdated timestamp.The data attribute correctly uses the provided
lastUpdatedvalue or falls back to the current timestamp, ensuring consistent timestamp tracking across the application.src/components/ItemsLoader.ts (1)
33-33: Good refactoring - moving data processing upstream.Removing the conditional " - Topic" suffix here and handling it in the data fetching modules is a better separation of concerns. The UI component should focus on presentation, not data transformation.
src/index.d.ts (1)
44-44: Correct type definition for lastUpdated property.The optional
lastUpdatedstring property aligns with the timestamp tracking feature implemented across the application.src/modules/audioErrorHandler.ts (2)
11-11: LGTM! Removal of enforcePiped from destructuring.This aligns with the broader refactoring to reduce
enforcePipedusage across the codebase.
13-13: Confirm piped‐stream error handling behaviorRemoving
enforcePipedfrom the early return insrc/modules/audioErrorHandler.tsmeans that when users have piped audio enabled (but neither HLS nor a custom instance), any load errors will now fall through into the proxy-switching and fallback logic rather than immediately notifying and stopping playback. We didn’t find any otherenforcePipedchecks in this module, so please verify that this change is intentional.• File: src/modules/audioErrorHandler.ts
• Line: 13 (if (HLS || customInstance))If piped streams should still short-circuit on error (like HLS/custom), re-add
|| enforcePipedto that condition; otherwise confirm that running through the full fallback flow is desired.src/modules/getStreamData.ts (2)
95-95: Good implementation of conditional local extraction.The dynamic import approach is appropriate for optional functionality. The async/await syntax is correct.
98-98: Local extraction module verified
- Found
src/modules/localExtraction.ts- Confirms it exports
export async function fetchDataFromLocal(id: string): Promise<Piped & { captions: []; videoStreams: [] }> { … }No further changes needed.
src/lib/libraryUtils.ts (2)
44-44: LGTM! Proper timestamp tracking implementation.The addition of
lastUpdatedtimestamp when saving collection items provides consistent tracking of when items were last modified. Using ISO string format is appropriate for localStorage serialization.
100-100: LGTM! Consistent timestamp propagation to UI components.The fallback to current timestamp ensures StreamItem always receives a valid
lastUpdatedvalue, maintaining consistency in the rendering pipeline.src/lib/utils.ts (1)
213-213: LGTM! Proper timestamp extraction from DOM dataset.The addition correctly extracts the
lastUpdatedtimestamp from the DOM element's dataset and provides a sensible fallback to the current timestamp, maintaining consistency with the actions menu state management.src/modules/fetchList.ts (2)
63-70: LGTM! Consistent uploader name transformation for Hyperpipe content.The conditional logic properly appends " - Topic" suffix to uploader names when using Hyperpipe, with appropriate checks to prevent duplicate suffixes. The implementation is clean and idempotent.
121-128: LGTM! Consistent suffix handling for paginated results.The same transformation logic is properly applied to paginated data, ensuring consistent uploader name formatting across both initial loads and subsequent pages.
src/modules/fetchSearchResults.ts (1)
114-116: LGTM! Proper suffix handling in paginated results.The conditional suffix addition is correctly implemented within the pagination loop, maintaining the existing filtering logic while consistently transforming uploader names for music content.
src/modules/jioSaavn.ts (1)
45-49: LGTM! Comprehensive error handling with graceful fallback.The error handling properly manages failures by updating the UI, disabling the JioSaavn feature, and falling back to the regular player. This ensures a good user experience even when JioSaavn integration fails.
src/lib/player.ts (3)
1-1: LGTM!Good cleanup removing the unnecessary aliasing of
titleasptitle.
25-30: Verify the JioSaavn activation logic.The current logic only imports the JioSaavn module when
store.player.useSaavnis already true AND the author ends with 'Topic'. This means on the first call whenuseSaavnis false, it will be set to true but the JioSaavn module won't be imported even if the author ends with 'Topic'. Is this the intended behavior?Consider this alternative structure if you want to import JioSaavn on the first call:
if (state.jiosaavn) { - if (!store.player.useSaavn) - store.player.useSaavn = true; - else if (store.stream.author.endsWith('Topic')) - return import('../modules/jioSaavn').then(mod => mod.default()); + store.player.useSaavn = true; + if (store.stream.author.endsWith('Topic')) + return import('../modules/jioSaavn').then(mod => mod.default()); }
32-32: LGTM!UI text updates are consistent with the import statement changes.
Also applies to: 40-40
src/modules/localExtraction.ts (3)
4-7: LGTM!Good error handling with appropriate fallback when JSON parsing fails.
9-12: LGTM!Appropriate validation of required fields while treating captions as optional.
49-62: LGTM!The return structure properly maps local data to the expected Piped interface format.
| export default function() { | ||
| title.textContent = 'Fetching Data via JioSaavn...'; | ||
| const { author, id } = store.stream; | ||
| const query = encodeURIComponent(`${store.stream.title.replace(/\(.*?\)/g, '')} ${author.slice(0, -8)}`); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Make author suffix removal more robust.
The hardcoded author.slice(0, -8) assumes the author always ends with " - Topic" (8 characters). This could incorrectly truncate author names that don't have this suffix, leading to poor search matches.
Apply this diff for more robust author processing:
- const query = encodeURIComponent(`${store.stream.title.replace(/\(.*?\)/g, '')} ${author.slice(0, -8)}`);
+ const authorName = author.endsWith(' - Topic') ? author.slice(0, -8) : author;
+ const query = encodeURIComponent(`${store.stream.title.replace(/\(.*?\)/g, '')} ${authorName}`);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const query = encodeURIComponent(`${store.stream.title.replace(/\(.*?\)/g, '')} ${author.slice(0, -8)}`); | |
| const authorName = author.endsWith(' - Topic') ? author.slice(0, -8) : author; | |
| const query = encodeURIComponent(`${store.stream.title.replace(/\(.*?\)/g, '')} ${authorName}`); |
🤖 Prompt for AI Agents
In src/modules/jioSaavn.ts at line 9, the current code uses a hardcoded slice to
remove the last 8 characters from the author string, assuming it always ends
with " - Topic". To fix this, replace the slice with a check that removes the
suffix only if it actually exists at the end of the author string. Use a
conditional or regex to detect and remove " - Topic" suffix safely, ensuring
author names without this suffix remain unchanged.
| const matchingTrack = res.data.results.find((track: { | ||
| name: string, | ||
| artists: { primary: { name: string }[] } | ||
| }) => | ||
| store.stream.title.startsWith(track.name) && author.startsWith(track.artists.primary[0].name) | ||
|
|
||
| ); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve track matching robustness and consistency.
Several concerns with the current matching logic:
- Inconsistent author processing: The query uses processed author name, but matching uses the original
authorvariable - Loose matching criteria: Using
startsWithfor both title and author could lead to false positives - Potential runtime error: No safety check for empty
track.artists.primaryarray
Apply this diff to improve matching:
+ const authorName = author.endsWith(' - Topic') ? author.slice(0, -8) : author;
const matchingTrack = res.data.results.find((track: {
name: string,
artists: { primary: { name: string }[] }
}) =>
- store.stream.title.startsWith(track.name) && author.startsWith(track.artists.primary[0].name)
+ track.name.toLowerCase() === store.stream.title.toLowerCase() &&
+ track.artists.primary.length > 0 &&
+ track.artists.primary[0].name.toLowerCase() === authorName.toLowerCase()
);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const matchingTrack = res.data.results.find((track: { | |
| name: string, | |
| artists: { primary: { name: string }[] } | |
| }) => | |
| store.stream.title.startsWith(track.name) && author.startsWith(track.artists.primary[0].name) | |
| ); | |
| const authorName = author.endsWith(' - Topic') ? author.slice(0, -8) : author; | |
| const matchingTrack = res.data.results.find((track: { | |
| name: string, | |
| artists: { primary: { name: string }[] } | |
| }) => | |
| track.name.toLowerCase() === store.stream.title.toLowerCase() && | |
| track.artists.primary.length > 0 && | |
| track.artists.primary[0].name.toLowerCase() === authorName.toLowerCase() | |
| ); |
🤖 Prompt for AI Agents
In src/modules/jioSaavn.ts around lines 16 to 22, improve the track matching
logic by ensuring consistent use of the processed author variable instead of the
original author, replace the loose startsWith checks with stricter equality or
more precise matching methods for both title and author to reduce false
positives, and add a safety check to confirm that track.artists.primary is not
empty before accessing its first element to prevent runtime errors.
| const { url, quality } = downloadUrl[{ | ||
| low: 1, | ||
| medium: downloadUrl.length - 2, | ||
| high: downloadUrl.length - 1 | ||
| }[state.quality]]; | ||
|
|
||
| audio.src = url.replace('http:', 'https:'); | ||
| qualityView.textContent = quality + ' AAC'; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add safety checks for quality selection.
The quality selection logic assumes specific array structure and length without validation. This could cause runtime errors if the downloadUrl array doesn't match expectations.
Apply this diff to add safety checks:
- const { url, quality } = downloadUrl[{
- low: 1,
- medium: downloadUrl.length - 2,
- high: downloadUrl.length - 1
- }[state.quality]];
+ const qualityIndex = {
+ low: Math.min(1, downloadUrl.length - 1),
+ medium: Math.max(0, downloadUrl.length - 2),
+ high: downloadUrl.length - 1
+ }[state.quality];
+
+ if (!downloadUrl[qualityIndex]) {
+ throw new Error('No audio stream available for selected quality');
+ }
+
+ const { url, quality } = downloadUrl[qualityIndex];
- audio.src = url.replace('http:', 'https:');
+ audio.src = url.startsWith('http:') ? url.replace('http:', 'https:') : url;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const { url, quality } = downloadUrl[{ | |
| low: 1, | |
| medium: downloadUrl.length - 2, | |
| high: downloadUrl.length - 1 | |
| }[state.quality]]; | |
| audio.src = url.replace('http:', 'https:'); | |
| qualityView.textContent = quality + ' AAC'; | |
| const qualityIndex = { | |
| low: Math.min(1, downloadUrl.length - 1), | |
| medium: Math.max(0, downloadUrl.length - 2), | |
| high: downloadUrl.length - 1 | |
| }[state.quality]; | |
| if (!downloadUrl[qualityIndex]) { | |
| throw new Error('No audio stream available for selected quality'); | |
| } | |
| const { url, quality } = downloadUrl[qualityIndex]; | |
| audio.src = url.startsWith('http:') ? url.replace('http:', 'https:') : url; | |
| qualityView.textContent = quality + ' AAC'; |
🤖 Prompt for AI Agents
In src/modules/jioSaavn.ts around lines 32 to 39, the code assumes the
downloadUrl array has a specific length and structure when selecting quality,
which can cause runtime errors if the array is shorter or differently
structured. Add safety checks to verify that the indices used for low, medium,
and high quality exist in the downloadUrl array before accessing them. If the
expected index is out of bounds, provide a fallback to a valid index or handle
the error gracefully to prevent crashes.
| export async function fetchDataFromLocal(id: string): Promise<Piped & { captions: [], videoStreams: [] }> { | ||
| const res = await fetch('http://localhost:9999/streams/' + id); |
There was a problem hiding this comment.
Fix return type declaration and consider making the URL configurable.
The return type { captions: [], videoStreams: [] } indicates empty arrays, but the function returns populated arrays. Also, the hardcoded localhost URL limits flexibility.
-export async function fetchDataFromLocal(id: string): Promise<Piped & { captions: [], videoStreams: [] }> {
- const res = await fetch('http://localhost:9999/streams/' + id);
+export async function fetchDataFromLocal(id: string): Promise<Piped & { captions: any[], videoStreams: any[] }> {
+ const localUrl = process.env.LOCAL_EXTRACTOR_URL || 'http://localhost:9999';
+ const res = await fetch(`${localUrl}/streams/${id}`);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export async function fetchDataFromLocal(id: string): Promise<Piped & { captions: [], videoStreams: [] }> { | |
| const res = await fetch('http://localhost:9999/streams/' + id); | |
| export async function fetchDataFromLocal(id: string): Promise<Piped & { captions: any[], videoStreams: any[] }> { | |
| const localUrl = process.env.LOCAL_EXTRACTOR_URL || 'http://localhost:9999'; | |
| const res = await fetch(`${localUrl}/streams/${id}`); |
🤖 Prompt for AI Agents
In src/modules/localExtraction.ts at lines 1 to 2, update the return type to
reflect that captions and videoStreams are arrays with elements, not empty
arrays. Replace the empty array types with appropriate array types indicating
the expected content. Additionally, refactor the fetch URL to be configurable,
for example by using an environment variable or a function parameter, instead of
hardcoding 'http://localhost:9999'.
| const audioStreams = streamingData.adaptiveFormats | ||
| ? streamingData.adaptiveFormats | ||
| .filter((f: any) => f.mimeType?.startsWith('audio')) | ||
| .map((f: any) => ({ | ||
| url: f.url, | ||
| bitrate: parseInt(f.bitrate), | ||
| codec: f.mimeType.includes('opus') ? 'opus' : (f.mimeType.includes('aac') ? 'aac' : 'unknown'), | ||
| contentLength: parseInt(f.contentLength || '0'), | ||
| quality: f.audioQuality || `${Math.floor(parseInt(f.bitrate) / 1024)} kbps`, | ||
| mimeType: f.mimeType, | ||
| })) | ||
| : []; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve type safety and codec detection.
The code uses any type and has limited codec detection. Also, the bitrate division by 1024 should be 1000 for kbps.
+interface AdaptiveFormat {
+ url: string;
+ mimeType: string;
+ bitrate: string;
+ contentLength?: string;
+ audioQuality?: string;
+ qualityLabel?: string;
+ quality?: string;
+ width?: number;
+ height?: number;
+}
+
const audioStreams = streamingData.adaptiveFormats
? streamingData.adaptiveFormats
- .filter((f: any) => f.mimeType?.startsWith('audio'))
- .map((f: any) => ({
+ .filter((f: AdaptiveFormat) => f.mimeType?.startsWith('audio'))
+ .map((f: AdaptiveFormat) => ({
url: f.url,
bitrate: parseInt(f.bitrate),
- codec: f.mimeType.includes('opus') ? 'opus' : (f.mimeType.includes('aac') ? 'aac' : 'unknown'),
+ codec: f.mimeType.match(/codecs="([^"]+)"/)?.[1] || f.mimeType.split('/')[1] || 'unknown',
contentLength: parseInt(f.contentLength || '0'),
- quality: f.audioQuality || `${Math.floor(parseInt(f.bitrate) / 1024)} kbps`,
+ quality: f.audioQuality || `${Math.floor(parseInt(f.bitrate) / 1000)} kbps`,
mimeType: f.mimeType,
}))
: [];📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const audioStreams = streamingData.adaptiveFormats | |
| ? streamingData.adaptiveFormats | |
| .filter((f: any) => f.mimeType?.startsWith('audio')) | |
| .map((f: any) => ({ | |
| url: f.url, | |
| bitrate: parseInt(f.bitrate), | |
| codec: f.mimeType.includes('opus') ? 'opus' : (f.mimeType.includes('aac') ? 'aac' : 'unknown'), | |
| contentLength: parseInt(f.contentLength || '0'), | |
| quality: f.audioQuality || `${Math.floor(parseInt(f.bitrate) / 1024)} kbps`, | |
| mimeType: f.mimeType, | |
| })) | |
| : []; | |
| interface AdaptiveFormat { | |
| url: string; | |
| mimeType: string; | |
| bitrate: string; | |
| contentLength?: string; | |
| audioQuality?: string; | |
| qualityLabel?: string; | |
| quality?: string; | |
| width?: number; | |
| height?: number; | |
| } | |
| const audioStreams = streamingData.adaptiveFormats | |
| ? streamingData.adaptiveFormats | |
| .filter((f: AdaptiveFormat) => f.mimeType?.startsWith('audio')) | |
| .map((f: AdaptiveFormat) => ({ | |
| url: f.url, | |
| bitrate: parseInt(f.bitrate), | |
| codec: f.mimeType.match(/codecs="([^"]+)"/)?.[1] || f.mimeType.split('/')[1] || 'unknown', | |
| contentLength: parseInt(f.contentLength || '0'), | |
| quality: f.audioQuality || `${Math.floor(parseInt(f.bitrate) / 1000)} kbps`, | |
| mimeType: f.mimeType, | |
| })) | |
| : []; |
🤖 Prompt for AI Agents
In src/modules/localExtraction.ts around lines 14 to 25, improve type safety by
replacing the use of 'any' with a proper interface or type for the
streamingData.adaptiveFormats elements. Enhance codec detection by expanding the
conditions to recognize more codecs beyond just 'opus' and 'aac'. Also, correct
the bitrate conversion by dividing by 1000 instead of 1024 when calculating kbps
in the quality field. Update the parsing of bitrate and contentLength to ensure
they are numbers and handle missing or malformed values gracefully.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/scripts/list.ts(1 hunks)src/scripts/queue.ts(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/scripts/queue.ts (1)
src/lib/store.ts (1)
store(66-160)
🔇 Additional comments (2)
src/scripts/list.ts (1)
33-34: LGTM! Dataset property normalization looks good.The explicit copying of snake_case dataset properties to camelCase versions ensures consistent property naming before appending to the queue. This aligns with the broader effort to standardize the
lastUpdatedproperty propagation across the application.src/scripts/queue.ts (1)
177-177: LGTM! Proper propagation oflastUpdatedproperty.The addition of the
lastUpdatedproperty to the StreamItem component aligns well with the broader effort to maintain consistent timestamp tracking across the application.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
vite.config.ts (1)
12-12: Consider simplifying the date formatting logic for better readability.The dynamic build version generation is a good improvement over the static string. However, the current implementation using an IIFE with default parameter assignment is quite complex and hard to read.
Consider extracting the date formatting logic for better readability:
- Build: JSON.stringify(((d = new Date()) => `v7x8 ${d.getFullYear().toString().slice(-2)}.${(d.getMonth() + 1).toString().padStart(2, '0')}.${d.getDate().toString().padStart(2, '0')}`)()) + Build: JSON.stringify((() => { + const now = new Date(); + const year = now.getFullYear().toString().slice(-2); + const month = (now.getMonth() + 1).toString().padStart(2, '0'); + const day = now.getDate().toString().padStart(2, '0'); + return `v7x8 ${year}.${month}.${day}`; + })())Alternatively, use a more concise approach:
- Build: JSON.stringify(((d = new Date()) => `v7x8 ${d.getFullYear().toString().slice(-2)}.${(d.getMonth() + 1).toString().padStart(2, '0')}.${d.getDate().toString().padStart(2, '0')}`)()) + Build: JSON.stringify(`v7x8 ${new Date().toLocaleDateString('en-CA').slice(2).replace(/-/g, '.')}`)Note: The current implementation uses local timezone, which means builds on different machines/timezones might produce different version strings on the same UTC date.
src/modules/getStreamData.ts (1)
98-98: Consider refactoring complex return logic for better readability.The nested ternary operators make this logic difficult to read and maintain. Consider refactoring to use if-else statements or a switch statement for better clarity.
- return (location.port === '9999') ? useLocal() : state.HLS ? useHls() : status === 'I' ? useInvidious() : status === 'N' ? fetchDataFromPiped(fallback) : usePiped(status === 'U' ? piped : proxy); + if (location.port === '9999') { + return useLocal(); + } + + if (state.HLS) { + return useHls(); + } + + switch (status) { + case 'I': + return useInvidious(); + case 'N': + return fetchDataFromPiped(fallback); + case 'U': + return usePiped(piped); + default: + return usePiped(proxy); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
index.html(1 hunks)package.json(1 hunks)src/components/Settings/playback.ts(0 hunks)src/components/UpdatePrompt.ts(1 hunks)src/lib/store.ts(4 hunks)src/lib/utils.ts(2 hunks)src/modules/fetchList.ts(5 hunks)src/modules/getStreamData.ts(3 hunks)src/modules/start.ts(1 hunks)src/scripts/queue.ts(1 hunks)vite.config.ts(1 hunks)
💤 Files with no reviewable changes (1)
- src/components/Settings/playback.ts
✅ Files skipped from review due to trivial changes (2)
- src/components/UpdatePrompt.ts
- index.html
🚧 Files skipped from review as they are similar to previous changes (5)
- src/lib/utils.ts
- src/lib/store.ts
- package.json
- src/scripts/queue.ts
- src/modules/fetchList.ts
🧰 Additional context used
🧬 Code Graph Analysis (2)
src/modules/getStreamData.ts (1)
src/lib/store.ts (2)
store(65-163)state(4-48)
src/modules/start.ts (1)
src/lib/store.ts (1)
store(65-163)
🔇 Additional comments (5)
src/modules/getStreamData.ts (3)
8-8: LGTM: Destructuring updated correctly.The addition of
proxyandstatusproperties aligns with the updated store structure and supports the enhanced API routing logic introduced later in the function.
73-78: Well-implemented function enhancement.The addition of the optional
srcparameter with a default value maintains backward compatibility while enabling the function to work with different API source arrays (piped or proxy). The recursive logic is correctly updated to pass thesrcparameter through.
95-95: Dynamic import implemented correctly.The use of dynamic import for local extraction functionality is appropriate for conditional loading and keeps the main bundle size smaller.
src/modules/start.ts (2)
18-22: Custom instance initialization updated correctly.The changes properly implement the new proxy/status-based API management:
- Setting both
proxy[0]andpiped[0]ensures consistency- Conditional invidious setup with status assignment aligns with the new routing logic
- Removal of deprecated
enforcePipedflag is consistent with the broader refactoring
32-32: LGTM: Proxy initialization added correctly.The addition of proxy array initialization from dynamic data is consistent with the existing pattern and necessary for the new proxy API support.
| document.querySelector('samp')!.textContent = { | ||
| U: "⬛⬛⬛⬛", P: "⬛⬛⬛⬜", I: "🟧⬛⬛⬜⬜", N: "⬛⬜⬜⬜" | ||
| }[data.health as 'U']; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve type safety for health status mapping.
The health status UI update is a good addition, but the type assertion as 'U' is unsafe and could cause runtime errors if data.health has an unexpected value.
- document.querySelector('samp')!.textContent = {
- U: "⬛⬛⬛⬛", P: "⬛⬛⬛⬜", I: "🟧⬛⬛⬜⬜", N: "⬛⬜⬜⬜"
- }[data.health as 'U'];
+ const healthSymbols = {
+ U: "⬛⬛⬛⬛", P: "⬛⬛⬛⬜", I: "🟧⬛⬛⬜⬜", N: "⬛⬜⬜⬜"
+ } as const;
+ const healthSymbol = healthSymbols[data.health as keyof typeof healthSymbols] || "❓❓❓❓";
+ document.querySelector('samp')!.textContent = healthSymbol;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| document.querySelector('samp')!.textContent = { | |
| U: "⬛⬛⬛⬛", P: "⬛⬛⬛⬜", I: "🟧⬛⬛⬜⬜", N: "⬛⬜⬜⬜" | |
| }[data.health as 'U']; | |
| const healthSymbols = { | |
| U: "⬛⬛⬛⬛", P: "⬛⬛⬛⬜", I: "🟧⬛⬛⬜⬜", N: "⬛⬜⬜⬜" | |
| } as const; | |
| const healthSymbol = healthSymbols[data.health as keyof typeof healthSymbols] || "❓❓❓❓"; | |
| document.querySelector('samp')!.textContent = healthSymbol; |
🤖 Prompt for AI Agents
In src/modules/start.ts around lines 27 to 29, the code uses an unsafe type
assertion 'as U' when accessing the health status mapping, which risks runtime
errors if data.health has unexpected values. To fix this, replace the assertion
with a type-safe check by validating that data.health is one of the expected
keys ('U', 'P', 'I', 'N') before accessing the mapping, and provide a fallback
value for unknown statuses to ensure safe assignment to textContent.
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
src/components/WatchVideo.ts (1)
128-138: Guard against empty Invidious proxy array in subtitle URLsWhile
store.api.invidiousis initialized insrc/modules/start.ts, there’s no guarantee its first element exists—if the array is empty,store.api.invidious[0]will beundefined, leading tosrcbecoming"undefined<subtitle-path>".• File: src/components/WatchVideo.ts
• Lines: 128–138Consider guarding or providing a fallback when the proxy list is empty:
- src=${(store.api.status === 'P' ? '' : store.api.invidious[0]) + v.url} + src=${( + store.api.status === 'P' || store.api.invidious.length === 0 + ? '' + : store.api.invidious[0] + ) + v.url}Or using the nullish coalescing operator:
- src=${(store.api.status === 'P' ? '' : store.api.invidious[0]) + v.url} + src=${( + store.api.status === 'P' + ? '' + : store.api.invidious[0] ?? '' + ) + v.url}This ensures
srcalways starts with a string, preventing"undefined"prefixes at runtime.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
netlify/edge-functions/fallback.ts(1 hunks)src/components/WatchVideo.ts(3 hunks)src/index.d.ts(3 hunks)src/modules/getStreamData.ts(5 hunks)src/modules/localExtraction.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- netlify/edge-functions/fallback.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- src/index.d.ts
- src/modules/localExtraction.ts
🧰 Additional context used
🧬 Code Graph Analysis (2)
src/components/WatchVideo.ts (1)
src/lib/store.ts (1)
store(65-163)
src/modules/getStreamData.ts (1)
src/lib/store.ts (2)
store(65-163)state(4-48)
🔇 Additional comments (6)
src/components/WatchVideo.ts (2)
35-40: Type definition updated correctly for codec field.The change from
typetocodecin thevideoStreamstype definition is consistent with the updated data structure. The codec detection logic on lines 38-39 properly uses the newcodecfield.
47-56: Codec field references updated consistently.All references to
f.typehave been correctly updated tof.codecthroughout the video stream filtering logic. The codec preference hierarchy (AV1 → VP9 → AVC) is maintained properly.src/modules/getStreamData.ts (4)
8-9: API destructuring expanded appropriately.The addition of
proxyandstatusto the destructuring aligns with the expandedstore.apistructure and supports the new conditional API selection logic.
36-39: Captions to subtitles transformation implemented correctly.The property rename and field mapping (
c.label→name,c.url→url) correctly transforms the Invidious captions structure to match the expected subtitles format used in the video component.
52-52: Field mapping for codec implemented correctly.The assignment
codec: v.typecorrectly maps the Invidioustypefield to the expectedcodecfield, maintaining compatibility with the video component's updated structure.
76-81: usePiped function refactored effectively for flexibility.The addition of the optional
srcparameter with a default value maintains backward compatibility while enabling the function to work with bothpipedandproxyarrays. The recursive logic correctly propagates the source parameter.
| const useLocal = async () => await import('./localExtraction.ts').then(mod => mod.fetchDataFromLocal(id)); | ||
|
|
||
|
|
||
| return state.HLS ? useHls() : state.enforcePiped ? usePiped() : useInvidious(); | ||
| return (location.port === '9999') ? useLocal() : state.HLS ? useHls() : status === 'I' ? useInvidious() : status === 'N' ? fetchDataFromPiped(fallback) : usePiped(status === 'U' ? piped : proxy); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Complex API selection logic needs refactoring for maintainability.
The comprehensive API selection strategy covers all scenarios well, but the single-line conditional on line 101 is difficult to read and maintain. Consider refactoring for clarity:
- return (location.port === '9999') ? useLocal() : state.HLS ? useHls() : status === 'I' ? useInvidious() : status === 'N' ? fetchDataFromPiped(fallback) : usePiped(status === 'U' ? piped : proxy);
+ // Development mode with local extraction
+ if (location.port === '9999') {
+ return useLocal();
+ }
+
+ // HLS mode
+ if (state.HLS) {
+ return useHls();
+ }
+
+ // API selection based on status
+ switch (status) {
+ case 'I': return useInvidious();
+ case 'N': return fetchDataFromPiped(fallback);
+ default: return usePiped(status === 'U' ? piped : proxy);
+ }Additionally, consider adding error handling for the dynamic import in useLocal():
- const useLocal = async () => await import('./localExtraction.ts').then(mod => mod.fetchDataFromLocal(id));
+ const useLocal = async () => {
+ try {
+ const mod = await import('./localExtraction.ts');
+ return mod.fetchDataFromLocal(id);
+ } catch (error) {
+ console.warn('Local extraction failed, falling back to remote APIs:', error);
+ return state.HLS ? useHls() : usePiped();
+ }
+ };📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const useLocal = async () => await import('./localExtraction.ts').then(mod => mod.fetchDataFromLocal(id)); | |
| return state.HLS ? useHls() : state.enforcePiped ? usePiped() : useInvidious(); | |
| return (location.port === '9999') ? useLocal() : state.HLS ? useHls() : status === 'I' ? useInvidious() : status === 'N' ? fetchDataFromPiped(fallback) : usePiped(status === 'U' ? piped : proxy); | |
| const useLocal = async () => { | |
| try { | |
| const mod = await import('./localExtraction.ts'); | |
| return mod.fetchDataFromLocal(id); | |
| } catch (error) { | |
| console.warn('Local extraction failed, falling back to remote APIs:', error); | |
| return state.HLS ? useHls() : usePiped(status === 'U' ? piped : proxy); | |
| } | |
| }; | |
| // Development mode with local extraction | |
| if (location.port === '9999') { | |
| return useLocal(); | |
| } | |
| // HLS mode | |
| if (state.HLS) { | |
| return useHls(); | |
| } | |
| // API selection based on status | |
| switch (status) { | |
| case 'I': | |
| return useInvidious(); | |
| case 'N': | |
| return fetchDataFromPiped(fallback); | |
| default: | |
| return usePiped(status === 'U' ? piped : proxy); | |
| } |
🤖 Prompt for AI Agents
In src/modules/getStreamData.ts around lines 98 to 101, the complex single-line
conditional for API selection is hard to read and maintain. Refactor this logic
into a clearer multi-branch if-else or switch statement to improve readability.
Also, add error handling to the dynamic import in useLocal() by wrapping the
import call in a try-catch block and handling import failures gracefully.
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
1 participant
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Style
Chores