v8.3

[n-ce]

Summary by CodeRabbit

• New Features

  • Gallery view for related artists/playlists; new Library subfeed and Search UI; "Load All" for playlists.

• Improvements

  • Redesigned app layout, responsive footer/miniplayer and updated icons; actions menu, list and playback controls refined with clearer loading indicators and proxy handling.

• Localization

  • Added Croatian, Bosnian and Serbian locales; various locale text updates.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

Introduces a standardized API response wrapper and replaces legacy YouTube Music/Last.fm fetchers with youtubei.js backend modules, migrates item types to TrackItem/YTItem/YTListItem, rewires many stores and UI components to the new types, restructures CSS/layout, and adds dev API middleware for vite.

Changes

Cohort / File(s)	Summary
API helper & new/updated routes api/_helper.ts, api/album.ts, api/artist.ts, api/channel.ts, api/gallery.ts, api/playlist.ts, api/search-suggestions.ts, api/search.ts, api/similar.ts, api/subfeed.ts	Add wrap(res, promise) helper and refactor routes to minimal parameter validation then delegate to backend handlers via wrap.
Deleted API routes api/artists.ts, api/suggestions.ts, api/tracks.ts	Remove legacy in-file route implementations (inlined fetching/parsing and Last.fm/YouTubeMusic-specific logic).
Backend: youtubei.js implementations & utils src/backend/getAlbum.ts, src/backend/getArtist.ts, src/backend/getChannel.ts, src/backend/getGallery.ts, src/backend/getPlaylist.ts, src/backend/getSearch.ts, src/backend/getSearchSuggestions.ts, src/backend/getSimilar.ts, src/backend/getSubFeed.ts, src/backend/utils.ts	Add new youtubei.js-based backend modules and shared utilities (client factory, mappers, thumbnail/duration parsing, published parsing).
Removed legacy backend modules & adapter added src/backend/localAdapter.ts, removed: src/backend/youtube_*, src/backend/get_youtube_song.ts, src/backend/lastfm_api.ts, src/backend/suggestions.ts, src/backend/subfeed.ts	Add local adapter for vite dev; delete multiple legacy YouTube Music / Last.fm fetch modules replaced by new backend.
Types & model unification src/types.d.ts	Introduce unified item model: TrackItem, YTItem, YTListItem and related list/album/artist/channel types; replace legacy collection/stream types.
Store & state migrations src/lib/stores/*.ts (app.ts, list.ts, navigation.ts, player.ts, queue.ts, search.ts)	Migrate stores to TrackItem/YTItem shapes, add player proxy, change getList signature and list state shape, update navigation params and default features.
List / player flow & modules src/features/List/*, src/features/Player/*, src/lib/modules/getRadio.ts, src/lib/modules/getStreamData.ts	Refactor list sorting/state, album detection, playlist loadAll, simplify stream fetching and proxy behavior, add Piped playlist→TrackItem mapping.
Component prop/type changes src/components/ListItem.tsx, src/components/StreamItem.tsx, src/components/ActionsMenu/*, src/components/MediaPartials/*, src/components/Queue/List.tsx, src/components/ActionsMenu/CollectionSelector.tsx	Update component props to new YTItem/YTListItem/TrackItem shapes; add async loading states and minor icon/class updates.
Feature removals & UI rework src/features/Home/* (Hub/Search/Library deletions), src/features/Queue/Standby.tsx removed, new: src/features/Library/*, src/features/Search/*	Remove Hub/Standby and many legacy home/search components; add rebuilt Library, Gallery, SubFeed, and Search UI components wired to new APIs.
Module removals & migrations src/lib/modules/* (many deleted/updated), src/lib/modules/libraryMigratorV5.ts	Delete many legacy fetch modules; add/getRadio and migrate modules to TrackItem; provide library migration to v5.
Utilities & config changes src/lib/utils/*, src/lib/utils/config.ts, src/lib/utils/library.ts, src/lib/utils/pure.ts	Remove fetchJson/fetchUma, add instances and syncLibrary/cleanseLibraryData, change config/drawer shapes (sortBy, lastMainFeature, libraryPlays).
CSS / styling overhaul src/styles/global.css, src/styles/layout.css, src/styles/patterns.css, src/styles/remixicon.css, src/styles/footer.css, removed: many legacy CSS files (header.css, MiniPlayer.css, ToggleSwitch.css, etc.)	Large styling refactor: new layout and patterns, icon mapping changes, removal of many legacy stylesheet blocks.
Locales src/locales/*.json (added: bs.json, cnr.json, hr.json, sr.json; many edits/removals)	Add several locale files, remove queue_standby_header across locales, and update/add translation keys (e.g., en:list_load_all, nl/de updates).
Dev/server tooling vite.config.ts, package.json	Add API middleware to serve /api/* in dev via localAdapter, bump package versions and add youtubei.js devDependency.
Misc small edits assorted files (netlify hash normalization, cloudSync iteration, many type annotation updates CollectionItem→TrackItem)	Minor behavioral fixes (email hash normalization), cloudSync localStorage iteration change, and widespread type migrations from CollectionItem to TrackItem.

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant API as Vercel Route (/api/...)
  participant Wrap as api/_helper.wrap
  participant Backend as src/backend/* handler
  participant YouTube as youtubei.js

  Client->>API: HTTP request (query params)
  API->>Wrap: validate params, call Wrap(res, Backend(params))
  Wrap->>Backend: invoke handler(params)
  Backend->>YouTube: youtubei.js calls / fetch data
  YouTube-->>Backend: returns normalized data
  Backend-->>Wrap: resolves Promise with payload
  Wrap->>Client: set Cache-Control header + JSON 200 or log + JSON 500

Loading

sequenceDiagram
  participant DevBrowser
  participant ViteDev
  participant Middleware as vite apiMiddleware
  participant Adapter as localAdapter
  participant Route as ./api/* handler

  DevBrowser->>ViteDev: request /api/...
  ViteDev->>Middleware: intercept /api/*
  Middleware->>Adapter: createLocalAdapter(Route)
  Adapter->>Route: call Vercel-style handler (mock req/res)
  Route-->>Adapter: respond via mocked VercelResponse
  Adapter-->>ViteDev: send HTTP response to DevBrowser

Loading

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~120 minutes

Possibly related PRs

• Translations update from Hosted Weblate #369: Localization additions/edits — overlaps with added/updated locale files (hr, sr, bs, cnr) and key changes.
• Ported Major Components to uhtml #298: UI component changes — overlaps on ListItem/StreamItem and related component prop/type updates.
• Cloud Sync v2 & Unified LocalStorage State Handling #306: Cloud-sync/local-storage changes — related to cloudSync and library sync rewiring introduced here.

Poem

🐇 I wrapped responses, hopped through types and code,
Replaced old fetch burrows with youtubei's road,
The Hub packed its carrots, Gallery found light,
Locales sang new, icons donned fresh sprite,
thump — the rabbit cheers: endpoints tucked in tight!

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 4.17% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check	❓ Inconclusive	The pull request title 'v8.3' is vague and does not clearly summarize the main changes in the changeset.	Replace the generic version number with a descriptive title that conveys the primary objective, such as 'Migrate to youtubei.js backend and refactor API architecture' or similar.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

• 📝 Generate docstrings (stacked PR)
• 📝 Generate docstrings (commit on current branch)

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch dev-next

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 17

Note

Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)

src/lib/utils/config.ts (1)

48-56: ⚠️ Potential issue | 🟠 Major

Stale localStorage data will cause runtime errors with missing new properties.

When a returning user has a saved drawer in localStorage from a previous version, JSON.parse(savedDrawer) on line 56 replaces the entire default object. New keys like lastMainFeature and libraryPlays will be undefined. Direct property access in library.ts (line 169: libraryPlays[id] = ...), search.ts (line 71: recentSearches.length), and other locations will throw at runtime.

The same issue affects config (lines 34–36) with missing sortBy and sortOrder.

Merge saved data with defaults to preserve new keys:

Suggested fix

 const savedDrawer = localStorage.getItem('drawer');
 if (savedDrawer)
-  drawer = JSON.parse(savedDrawer);
+  drawer = { ...drawer, ...JSON.parse(savedDrawer) };

 const savedStore = localStorage.getItem('config');
 if (savedStore)
-  config = JSON.parse(savedStore);
+  config = { ...config, ...JSON.parse(savedStore) };

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/lib/utils/config.ts` around lines 48 - 56, The saved localStorage parse
is replacing the default objects (drawer and config) wholesale which drops newly
added keys; instead, after reading savedDrawer (and savedConfig) merge the
parsed value into the existing default objects so missing keys are preserved —
e.g., parse savedDrawer and then Object.assign(drawer, parsed) or use spread to
copy parsed properties onto the existing drawer object (same for config),
ensuring types are retained and that drawer.recentSearches, drawer.discovery,
drawer.lastMainFeature and drawer.libraryPlays (and
config.sortBy/config.sortOrder) exist after merging rather than being
overwritten by an older saved object.

src/features/List/Dropdown.tsx (1)

85-91: ⚠️ Potential issue | 🟡 Minor

Potential crash if fullList is empty — fullList[0] will be undefined.

Both getCollectionItems(listStore.id) and listStore.list can return empty arrays. Accessing fullList[0] on line 89–90 without a length check will pass undefined to setPlayerStore and player().

Proposed fix

          onclick={() => {
            setQueueStore('list', []);
            const fullList = listStore.type === 'collection' ? getCollectionItems(listStore.id) : listStore.list;
+           if (fullList.length === 0) return;
            addToQueue(fullList);
            setPlayerStore('stream', fullList[0]);
            player(fullList[0].id);
            setNavStore('queue', 'state', true);
          }}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/features/List/Dropdown.tsx` around lines 85 - 91, The onclick handler
builds fullList via getCollectionItems(listStore.id) or listStore.list and then
immediately uses fullList[0]; add a guard to handle empty arrays: after
computing fullList, check if fullList.length === 0 and if so avoid calling
setPlayerStore('stream', ...) and player(...), and decide an alternative (e.g.,
early return, set an empty stream state, or show a message) instead of passing
undefined; ensure addToQueue(fullList) and setQueueStore('list', []) behavior
remains correct and only call setNavStore('queue','state', true) when
appropriate, updating the onclick block around setQueueStore, addToQueue,
setPlayerStore, player, and setNavStore to implement this guard.

🟡 Minor comments (29)

src/locales/bs.json-154-154 (1)

154-154: ⚠️ Potential issue | 🟡 Minor

Incorrect translation for settings_quality_worst.

"Biću manje pripremljena" ("I will be less prepared") is a grammatically unrelated sentence and doesn't convey audio quality. The expected value should be a short-form equivalent of "Worst", e.g., "Najgora".

🌐 Suggested fix

-    "settings_quality_worst": "Biću manje pripremljena",
+    "settings_quality_worst": "Najgora",

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/locales/bs.json` at line 154, Replace the incorrect Bosnian translation
string for the key settings_quality_worst by changing its value from "Biću manje
pripremljena" to a short adjective meaning "Worst" (e.g., "Najgora") so the key
settings_quality_worst correctly conveys audio quality rather than an unrelated
sentence.

src/features/Updater/Updater.css-29-30 (1)

29-30: ⚠️ Potential issue | 🟡 Minor

Stylelint error: add an empty line before the declaration following the nested rule.

The declaration-empty-line-before rule requires an empty line separating margin-bottom from the preceding &:last-child closing brace.

🔧 Proposed fix

     &:last-child {
       font-weight: var(--font-weight-5);
       border-bottom: 0;
     }
+
     margin-bottom: var(--size-2);
     border-bottom: var(--border);

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/features/Updater/Updater.css` around lines 29 - 30, The stylelint error
is caused by missing an empty line before the declaration that follows the
nested rule &:last-child; update the CSS around the &:last-child block so there
is a blank line between the closing brace of &:last-child and the subsequent
declaration (the margin-bottom line), ensuring a single empty line separates the
nested rule and the following declarations like margin-bottom and border-bottom.

src/locales/cnr.json-224-224 (1)

224-224: ⚠️ Potential issue | 🟡 Minor

Incorrect translation for player_video_resolution

"Odluka" means "Decision/Choice" in Croatian/Bosnian. The correct translation is "Rezolucija".

🔤 Proposed fix

-    "player_video_resolution": "Odluka"
+    "player_video_resolution": "Rezolucija"

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/locales/cnr.json` at line 224, The translation for the JSON key
"player_video_resolution" is incorrect ("Odluka"); update the value for the
"player_video_resolution" entry to the correct Croatian/Bosnian word
"Rezolucija" so the JSON reads with the same key but the corrected translation
string.

src/locales/hr.json-215-215 (1)

215-215: ⚠️ Potential issue | 🟡 Minor

Typo: "Konaflikt" → "Konflikt"

"sync_conflict" starts with "Konaflikt" — transposed letters. Should be "Konflikt".

🔤 Proposed fix

-    "sync_conflict": "Konaflikt: Biblioteka je ažurirana na drugom uređaju. Molimo ponovno sinkronizirajte.",
+    "sync_conflict": "Konflikt: Biblioteka je ažurirana na drugom uređaju. Molimo ponovno sinkronizirajte.",

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/locales/hr.json` at line 215, The translation value for the
"sync_conflict" key contains a typo ("Konaflikt"); update the string value for
the "sync_conflict" entry in src/locales/hr.json to "Konflikt: Biblioteka je
ažurirana na drugom uređaju. Molimo ponovno sinkronizirajte." so the word
"Konaflikt" is corrected to "Konflikt".

src/locales/cnr.json-168-168 (1)

168-168: ⚠️ Potential issue | 🟡 Minor

Broken $ placeholder — runtime substitution will not work

The $ runtime placeholder in settings_clear_discoveries was mistakenly replaced with the literal text "od 1 dolara". The app's template engine looks for $ to insert the discovery count; without it the number will not appear.

Compare with the correct form in hr.json:

"settings_clear_discoveries": "Ovo će otkloniti vaša postojeća $ otkrića, nastavite li?"

🐛 Proposed fix

-    "settings_clear_discoveries": "Ovo će otkloniti vaše postojeće otkriće od 1 dolara, nastaviti?",
+    "settings_clear_discoveries": "Ovo će otkloniti vaša postojeća $ otkrića, nastaviti?",

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/locales/cnr.json` at line 168, The localized string for the key
settings_clear_discoveries incorrectly includes the literal "od 1 dolara"
instead of the runtime placeholder; update the value of
settings_clear_discoveries to include the $ placeholder (e.g., "Ovo će otkloniti
vaša postojeća $ otkrića, nastavite li?") so the template engine can substitute
the discovery count at runtime while preserving grammar and punctuation.

src/locales/hr.json-175-175 (1)

175-175: ⚠️ Potential issue | 🟡 Minor

Typo: "Světllije" → "Svjetlije"

"settings_roundness_lighter" has a double l — "Světllije" should be "Svjetlije".

🔤 Proposed fix

-    "settings_roundness_lighter": "Světllije",
+    "settings_roundness_lighter": "Svjetlije",

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/locales/hr.json` at line 175, Fix the typo in the Croatian locale entry
"settings_roundness_lighter" by replacing the current value with the correct
spelling "Svjetlije" (change the double "l" form "Světllije"/"Svjetllije" to
"Svjetlije") in the JSON value for the key settings_roundness_lighter.

src/locales/cnr.json-154-154 (1)

154-154: ⚠️ Potential issue | 🟡 Minor

Incorrect translation for settings_quality_worst

"Biću manje pripremljena" translates to "I will be less prepared" — this is unrelated to audio quality. Expected value should be something like "Najgore" (as used in hr.json).

🔤 Proposed fix

-    "settings_quality_worst": "Biću manje pripremljena",
+    "settings_quality_worst": "Najgore",

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/locales/cnr.json` at line 154, The translation for the locale key
settings_quality_worst is incorrect ("Biću manje pripremljena"); replace its
value with the correct audio-quality term used in other locales (e.g.,
"Najgore") so the key settings_quality_worst correctly reflects "worst" audio
quality; update the JSON entry for settings_quality_worst accordingly and ensure
the surrounding JSON remains valid (commas/quotes intact).

src/styles/patterns.css-123-135 (1)

123-135: ⚠️ Potential issue | 🟡 Minor

white-space: pre on .displayer will overflow for dynamic content

white-space: pre prevents any line-wrapping, so long track titles, error messages, or playlist names will overflow the modal boundary horizontally. For a general-purpose displayer, pre-wrap preserves intentional line breaks while still wrapping long lines.

🐛 Proposed fix

 .displayer {
-  white-space: pre;
+  white-space: pre-wrap;

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/styles/patterns.css` around lines 123 - 135, The .displayer rule
currently uses white-space: pre which prevents wrapping and causes horizontal
overflow for long dynamic content; update the .displayer selector to use
white-space: pre-wrap and add an overflow wrapping rule (e.g., overflow-wrap:
anywhere or word-break: break-word) to allow long words/strings to wrap within
the modal while still preserving intentional line breaks; make these changes
inside the .displayer block shown in the diff so wrapping behavior applies
consistently.

vite.config.ts-142-164 (1)

142-164: ⚠️ Potential issue | 🟡 Minor

Guideline violation (async/await + try/catch) and uncaught error path

Two related issues:

1. async/await and try/catch violate the project guideline; .then().catch() is preferred.
2. The createLocalAdapter ssrLoadModule call (Line 147) sits outside the try/catch, so if it rejects (e.g., module not found), the error propagates to Vite's own error handler rather than calling next(). The dev request can hang or return a non-descriptive 500.

Switching to a .then().catch() chain fixes both issues and also replaces the deprecated url.parse call:

♻️ Proposed fix

-const apiMiddleware = (serve: boolean): PluginOption => serve ? {
-  name: 'api-middleware',
-  configureServer(server) {
-    server.middlewares.use(async (req, res, next) => {
-      if (req.url?.startsWith('/api/')) {
-        const { createLocalAdapter } = await server.ssrLoadModule('./src/backend/localAdapter.ts');
-        const parsedUrl = url.parse(req.url, true);
-        const endpoint = parsedUrl.pathname?.split('/').pop();
-
-        try {
-          const handlerModule = await server.ssrLoadModule(`./api/${endpoint}.ts`);
-          const adapter = createLocalAdapter(handlerModule.default);
-          return adapter(req, res);
-        } catch (e) {
-          console.error(`Failed to load API handler for ${endpoint}:`, e);
-          next();
-        }
-      } else {
-        next();
-      }
-    });
-  }
-} : [];
+const apiMiddleware = (serve: boolean): PluginOption => serve ? {
+  name: 'api-middleware',
+  configureServer(server) {
+    server.middlewares.use((req, res, next) => {
+      if (!req.url?.startsWith('/api/')) return next();
+      const endpoint = new URL(req.url, 'http://localhost').pathname.split('/').pop();
+      server.ssrLoadModule('./src/backend/localAdapter.ts')
+        .then(({ createLocalAdapter }) =>
+          server.ssrLoadModule(`./api/${endpoint}.ts`)
+            .then(mod => createLocalAdapter(mod.default)(req, res))
+        )
+        .catch(e => {
+          console.error(`Failed to load API handler for ${endpoint}:`, e);
+          next();
+        });
+    });
+  }
+} : [];

As per coding guidelines: "Prefer Promise-based .then().catch() syntax over async/await or try...catch blocks."

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@vite.config.ts` around lines 142 - 164, The apiMiddleware currently uses
async/await and a try/catch which violates our guideline and also calls
server.ssrLoadModule('./src/backend/localAdapter.ts') outside the error-handling
path so rejections can leak; refactor the middleware to use Promise chains
(.then().catch()) instead of async/await, move both server.ssrLoadModule calls
(the one for './src/backend/localAdapter.ts' and the one for
`./api/${endpoint}.ts`) into the same promise chain so any module load error is
caught and calls next(), and replace the deprecated url.parse usage with the
modern URL API (e.g., new URL(req.url, base)) when extracting pathname/endpoint;
keep references to apiMiddleware, createLocalAdapter, server.ssrLoadModule,
handlerModule, and adapter when applying these changes.

package.json-29-29 (1)

29-29: ⚠️ Potential issue | 🟡 Minor

Move youtubei.js from devDependencies to dependencies.

The backend modules (getSearch, getArtist, getChannel, etc.) import youtubei.js at runtime within Vercel serverless functions. While Vercel bundles these functions at build time (resolving all imports), placing runtime dependencies in devDependencies violates npm best practices and creates portability risk. If the code is deployed to a platform that only installs dependencies at runtime, or if someone runs npm ci --production locally to test, imports will fail. Moving youtubei.js to dependencies ensures correct behavior across all deployment scenarios.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@package.json` at line 29, The package "youtubei.js" is listed under
devDependencies but is required at runtime by serverless modules like
getSearch/getArtist/getChannel; move "youtubei.js" from devDependencies into
dependencies in package.json so it is installed in production builds and during
npm ci --production, ensuring runtime imports resolve across deployment
environments.

src/styles/footer.css-44-45 (1)

44-45: ⚠️ Potential issue | 🟡 Minor

Stylelint: missing empty line before declaration at line 45.

Static analysis reports declaration-empty-line-before at line 45. Add an empty line before width: var(--navW); to satisfy the linting rule.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/styles/footer.css` around lines 44 - 45, The CSS rule defining the custom
property --navW and the subsequent width declaration violates the stylelint
declaration-empty-line-before rule; to fix, insert a single blank line between
the --navW declaration and the width: var(--navW); declaration so there is an
empty line before the width property (i.e., ensure the block contains "--navW:
calc(var(--size-8) * 3);" followed by a blank line and then "width:
var(--navW);").

src/styles/footer.css-1-6 (1)

1-6: ⚠️ Potential issue | 🟡 Minor

Typo in comment: "landcape" → "landscape".

Line 3: in landcape show separately, should be in landscape show separately,.

Fix

-in landcape show separately,
+in landscape show separately,

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/styles/footer.css` around lines 1 - 6, Fix the typo in the top-of-file
comment in footer.css: replace "landcape" with "landscape" in the comment line
that currently reads "in landcape show separately," so it becomes "in landscape
show separately,"; no other changes required.

src/features/Search/Filters.tsx-22-22 (1)

22-22: ⚠️ Potential issue | 🟡 Minor

value={config.searchFilter} is non-reactive in SolidJS — select will desync if filter changes externally.

In SolidJS, components run only once; reading a plain object property outside of a reactive primitive does not register reactivity. config is a plain let object (not a createStore or createSignal), so value={config.searchFilter} is evaluated once at mount. If config.searchFilter is updated programmatically after mount (e.g., from URL parameter initialization on navigation), the select will display a stale value while applying a different filter. Wrap config.searchFilter in a reactive accessor or back it with a SolidJS signal/store.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/features/Search/Filters.tsx` at line 22, The select's value is bound to a
non-reactive plain object property (config.searchFilter) so it won't update when
changed; make it reactive by backing config.searchFilter with a SolidJS signal
or store (e.g., createSignal or createStore) and use its accessor in the JSX
(value={searchFilter()} or value={config.searchFilter} from a store) so the
select stays in sync when updated externally; update references in Filters.tsx
where value={config.searchFilter} is used and ensure updates use the
signal/store setter or mutate the store via its updater function.

api/playlist.ts-5-10 (1)

5-10: ⚠️ Potential issue | 🟡 Minor

Missing HTTP method guard — same pattern as api/channel.ts.

Consistent with the fix suggested for api/channel.ts, add a 405 guard for non-GET requests.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@api/playlist.ts` around lines 5 - 10, The exported async default handler
currently accepts any HTTP method; add the same 405 guard used in api/channel.ts
by checking req.method !== 'GET' near the top of the default function and
returning res.status(405).setHeader('Allow', 'GET').json({ error: 'Method Not
Allowed' }); before any other logic, then continue to validate id and call
wrap(res, handler(id)) as before; reference the default exported async function,
req.method, handler, and wrap to locate where to insert the guard.

api/artist.ts-5-10 (1)

5-10: ⚠️ Potential issue | 🟡 Minor

Missing HTTP method guard — same inconsistency as the other new routes.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@api/artist.ts` around lines 5 - 10, The exported async route handler in
api/artist.ts lacks an HTTP method guard; update the exported default async
function to check req.method and only allow GET (or the intended method),
returning a 405 status and setting the Allow header for disallowed methods,
before proceeding to validate id and call handler(id) wrapped via wrap(res,
handler(id)); place the method-check early in the function to prevent running
handler for unsupported methods.

src/features/Search/Filters.tsx-17-17 (1)

17-17: ⚠️ Potential issue | 🟡 Minor

Redundant updateParam('f', value) — getSearchResults() already handles this correctly.

getSearchResults internally calls updateParam('f', searchFilter === 'all' ? '' : searchFilter), which correctly deletes the param when 'all' is selected. Line 17 instead always sets ?f=all for the 'all' case, causing a brief incorrect URL state before getSearchResults cleans it up and a double history.replaceState call. Remove line 17 and let getSearchResults be the sole owner of URL state for f.

♻️ Suggested fix

         setConfig('searchFilter', value);
-        updateParam('f', value);
 
         getSearchResults();

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/features/Search/Filters.tsx` at line 17, Remove the redundant call to
updateParam('f', value) so URL state for the filter is only managed by
getSearchResults; specifically, in the handler/function that currently calls
updateParam('f', value) (the line shown), delete that line and rely on
getSearchResults(searchFilter) which already calls updateParam('f', searchFilter
=== 'all' ? '' : searchFilter) to set or remove the f param and perform
history.replaceState once.

src/features/Library/SubLists.tsx-38-38 (1)

38-38: ⚠️ Potential issue | 🟡 Minor

Unsafe (pl as Playlist).author — silently undefined for channel entries.

When flag === 'channels', pl comes from getLists('channels'). Channel objects likely don't carry an author field matching the Playlist interface, so the cast produces undefined without a compile-time error, breaking the author display for all channel items. Prefer an optional access with a fallback:

🛡️ Suggested fix

-      author: (pl as Playlist).author,
+      author: (pl as { author?: string }).author ?? '',

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/features/Library/SubLists.tsx` at line 38, The code unsafely casts pl to
Playlist with (pl as Playlist).author which yields undefined for channel
entries; update the mapping that sets author to use safe optional access and a
fallback based on the list type or channel shape — e.g., if flag === 'channels'
read the channel's author/owner field (or use optional chaining like pl?.author
?? pl?.owner) and provide a default string (e.g., 'Unknown') so channel objects
don't produce an undefined author; locate the mapping that assigns author (uses
pl variable and Playlist type) and replace the direct cast with a
conditional/optional-access fallback using getLists('channels') handling.

api/channel.ts-5-10 (1)

5-10: ⚠️ Potential issue | 🟡 Minor

Missing HTTP method guard — inconsistent with api/artists.ts.

The existing api/artists.ts explicitly rejects non-GET requests with a 405. These new routes accept any HTTP verb (POST, PUT, DELETE) and silently process them, which is inconsistent and a minor security posture gap.

🛡️ Suggested fix

 export default async function (req: VercelRequest, res: VercelResponse) {
+  if (req.method !== 'GET') {
+    res.setHeader('Allow', ['GET']);
+    return res.status(405).end(`Method ${req.method} Not Allowed`);
+  }
   const { id } = req.query;

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@api/channel.ts` around lines 5 - 10, The route handler in the default export
accepts any HTTP method; add an explicit guard to only allow GET and return 405
for others (mirroring api/artists.ts). Inside the exported async function (the
default handler that reads req.query.id and calls wrap(res, handler(id))), check
req.method !== 'GET' and respond with res.setHeader('Allow', 'GET') and
res.status(405).json({ error: 'Method Not Allowed' }) before proceeding to
validate id and call handler(id).

src/features/Search/Filters.tsx-11-15 (1)

11-15: ⚠️ Potential issue | 🟡 Minor

Results cleared unconditionally before getSearchResults() performs its empty-query guard.

If the user changes the filter while the search box is empty, searchStore.observer.disconnect() and setSearchStore({ page: 1, results: [] }) run, wiping any existing results. Then getSearchResults() returns early (if (!query) return), leaving the user with an empty, non-loading view. Consider guarding the reset on a non-empty query:

🛡️ Suggested fix

       onchange={(e) => {
         const { value } = e.target;
-        searchStore.observer.disconnect();
-        setSearchStore({
-          page: 1,
-          results: [],
-        });
         setConfig('searchFilter', value);
         getSearchResults();
       }}

(Let getSearchResults own the observer disconnect and results reset, since it already does both and respects the empty-query guard.)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/features/Search/Filters.tsx` around lines 11 - 15, The filter change
handler currently unconditionally calls searchStore.observer.disconnect() and
setSearchStore({ page: 1, results: [] }), which wipes results even when the
search box is empty and getSearchResults() will return early; instead, guard the
reset/disconnect behind a non-empty query or move those calls into
getSearchResults() so they only run when a real query is being fetched — update
the code to let getSearchResults() own observer.disconnect() and setSearchStore
reset logic (or wrap the existing disconnect/reset with if (query) ) and remove
the unconditional calls from the filter change path to preserve results when
query is empty.

src/features/Library/Gallery.tsx-41-44 (1)

41-44: ⚠️ Potential issue | 🟡 Minor

Missing res.ok check — HTTP error responses are silently cast to the gallery shape.

If the API returns a non-2xx response, res.json() will parse the error body { error: '...' } which then gets forced into { userArtists, relatedArtists, relatedPlaylists } via the type assertion. setGallery stores the corrupted value, and the ?.length > 0 guards in the JSX may produce unexpected rendering.

🐛 Proposed fix

-      const res = await fetch(`${store.api}/api/gallery?id=${artistIds.join(',')}`);
-      const data = await res.json() as { userArtists: Channel[], relatedArtists: Channel[], relatedPlaylists: Playlist[] };
-      setGallery(data);
+      const res = await fetch(`${store.api}/api/gallery?id=${artistIds.join(',')}`);
+      if (!res.ok) throw new Error(`Gallery API error: ${res.status}`);
+      const data = await res.json() as { userArtists: Channel[], relatedArtists: Channel[], relatedPlaylists: Playlist[] };
+      setGallery(data);

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/features/Library/Gallery.tsx` around lines 41 - 44, The fetch block in
Gallery.tsx is missing a check for non-2xx responses so res.json() may be cast
into the gallery shape and passed to setGallery; update the try block that calls
fetch(`${store.api}/api/gallery?id=${artistIds.join(',')}`) to verify res.ok
before parsing — if !res.ok, throw or handle an error (include status/text)
instead of calling setGallery, and only call setGallery with the parsed payload
when the response is OK; ensure this uses the same fetch/res variables and
preserves existing error handling.

src/features/Library/Dropdown.tsx-92-97 (1)

92-97: ⚠️ Potential issue | 🟡 Minor

Hardcoded English string — use t() for i18n consistency.

Line 94: "Import Playlists from SongShift" is not wrapped in a translation call, unlike every other menu label in this component. This will render untranslated for non-English users.

Proposed fix

-            <i class="ri-refresh-line"></i>&nbsp;Import Playlists from SongShift
+            <i class="ri-refresh-line"></i>&nbsp;{t('library_import_songshift')}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/features/Library/Dropdown.tsx` around lines 92 - 97, The label text
"Import Playlists from SongShift" in Dropdown.tsx is hardcoded; replace it with
a translation call (e.g., t('library.importFromSongshift') or the appropriate
key used in this component) so it follows the same i18n pattern as other menu
items; update the label node containing the icon and text (the element near id
'upload_songshift' and the <li> with onclick) to use t(...) instead of the raw
English string and ensure the translation key is added to your locale files.

src/backend/getSearchSuggestions.ts-10-11 (1)

10-11: ⚠️ Potential issue | 🟡 Minor

Potential NPE if contents is nullish on a non-null suggestion.

suggestions.first()?.contents only guards against first() being nullish. If the first suggestion exists but its contents property is undefined or null, calling .map() will throw.

Proposed fix — add optional chaining on `contents`

-    return suggestions.first()?.contents
-      .map(s => (s.as(YTNodes.SearchSuggestion)).suggestion.toString()) || [];
+    return suggestions.first()?.contents
+      ?.map(s => (s.as(YTNodes.SearchSuggestion)).suggestion.toString()) ?? [];

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/backend/getSearchSuggestions.ts` around lines 10 - 11, The return
expression in getSearchSuggestions uses suggestions.first()?.contents.map(...)
which will throw if the first suggestion exists but its contents is
null/undefined; update the expression to safely handle nullish contents (e.g.,
use optional chaining and a fallback array) so that you call .map only on an
array — adjust the expression that references suggestions.first(), .contents,
YTNodes.SearchSuggestion and .suggestion to use ?.contents and default to []
before mapping and converting to string.

src/features/Library/index.tsx-80-84 (1)

80-84: ⚠️ Potential issue | 🟡 Minor

Hardcoded English strings in aria-label — breaks i18n.

The sync button aria-labels ('Save to Cloud', 'Import from Cloud', 'Syncing') are hardcoded in English instead of using the t() translation function that is already imported and used elsewhere in this file.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/features/Library/index.tsx` around lines 80 - 84, The aria-label values
for the sync button are hardcoded in English; update the conditional expression
that sets aria-label to use the i18n translator function t() instead of string
literals (referencing store.syncState and the aria-label prop). Replace 'Save to
Cloud', 'Import from Cloud', and 'Syncing' with calls like t('...') using the
existing translation keys (or add new keys) so the aria-label becomes something
like aria-label={store.syncState === 'dirty' || store.syncState === 'error' ?
t('sync.saveToCloud') : store.syncState === 'synced' ? t('sync.importFromCloud')
: t('sync.syncing')} ensuring you import/use the same t function already in this
file.

src/components/ActionsMenu/index.tsx-31-33 (1)

31-33: ⚠️ Potential issue | 🟡 Minor

Unsafe cast — destructuring undefined will throw at runtime.

If store.actionsMenu is undefined (set on line 19 via closeDialog), the destructure const { id } = store.actionsMenu as TrackItem will throw a TypeError. The as TrackItem is compile-time only.

Proposed fix

-    const { id } = store.actionsMenu as TrackItem;
-    if (id)
+    const id = store.actionsMenu?.id;
+    if (id)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/components/ActionsMenu/index.tsx` around lines 31 - 33, The code
destructures id from store.actionsMenu with a cast to TrackItem which will throw
if actionsMenu is undefined; change this to safely access the id (e.g., check
store.actionsMenu exists before destructuring or use optional chaining) and only
call setIsListenLater(getCollection('listenLater').includes(id)) when id is
defined; update references around store.actionsMenu, TrackItem, setIsListenLater
and closeDialog so the component guards against undefined actionsMenu at
runtime.

src/features/Library/index.tsx-104-112 (1)

104-112: ⚠️ Potential issue | 🟡 Minor

Icon class conflict — both ri-user-heart-line and ri-user-heart-fill will be applied simultaneously.

The static class="ri-user-heart-line" is always present, and classList adds ri-user-heart-fill on top when showGallery() is true. Unlike the SubFeed toggle on line 97 which correctly uses a ternary to swap classes, this will render both icon classes together.

Proposed fix — use the same pattern as the SubFeed toggle

          <i
             aria-label={t('hub_gallery')}
-            class="ri-user-heart-line"
-            classList={{ 'ri-user-heart-fill': showGallery() }}
+            class={`ri-user-heart-${showGallery() ? 'fill' : 'line'}`}
             onclick={() => {

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/features/Library/index.tsx` around lines 104 - 112, The icon is receiving
both static class "ri-user-heart-line" and conditional "ri-user-heart-fill" via
classList causing class conflict; update the <i> element to use the same ternary
swap pattern as the SubFeed toggle so only one of "ri-user-heart-line" or
"ri-user-heart-fill" is applied based on showGallery(), keep the onclick handler
using setShowGallery and setShowSubFeed as-is (refer to showGallery(),
setShowGallery(), setShowSubFeed() and the SubFeed toggle implementation) to
locate and mirror the correct class assignment.

src/features/Library/index.tsx-87-91 (1)

87-91: ⚠️ Potential issue | 🟡 Minor

Unhandled promise rejection on cloud sync.

The dynamic import and runSync call has no .catch() handler. If the import fails or runSync throws, the error will be an unhandled promise rejection.

Proposed fix

              onclick={() => {
                import('@lib/modules/cloudSync').then(({ runSync }) => {
                  runSync(config.dbsync);
-                });
+                }).catch(err => {
+                  setStore('snackbar', err instanceof Error ? err.message : 'Sync failed');
+                });
              }}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/features/Library/index.tsx` around lines 87 - 91, The onclick handler
calls import('@lib/modules/cloudSync') and invokes runSync(config.dbsync)
without handling failures; wrap the dynamic import and the runSync invocation in
a promise error handler (or convert the handler to async and use try/catch) so
that both import() rejections and runSync errors are caught and logged/handled;
specifically update the onclick block that imports '@lib/modules/cloudSync' and
calls runSync to include a .catch(...) (or try/catch around await import(...)
and await runSync(...)) that handles the error (e.g., processLogger.error or
user-visible feedback).

src/backend/getGallery.ts-91-99 (1)

91-99: ⚠️ Potential issue | 🟡 Minor

Single-artist inputs will always return empty relatedArtists and relatedPlaylists.

Both relatedArtists (line 92) and relatedPlaylists (line 97) filter for count > 1, meaning an item must appear across at least two different input artists. When ids contains a single artist, every aggregated item will have count === 1 and be filtered out, so the caller always gets empty arrays for single-artist galleries. If this is intentional, a code comment would help clarify the contract; otherwise, consider using count >= 1 or adjusting the threshold based on ids.length.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/backend/getGallery.ts` around lines 91 - 99, The current filters on
recommendedArtistMap and relatedPlaylistMap use count > 1 which causes
relatedArtists and relatedPlaylists to be empty for single-item inputs; update
the filter logic in the block that builds relatedArtists and relatedPlaylists to
use a dynamic threshold based on the input ids (e.g., use count > 1 when
ids.length > 1, otherwise use count >= 1) or switch to count >= 1 if the intent
is to include any related item for single-artist galleries; adjust the filtering
expressions that reference recommendedArtistMap, relatedPlaylistMap and ids
accordingly and add a short comment near relatedArtists/relatedPlaylists
explaining the chosen rule.

src/backend/utils.ts-51-54 (1)

51-54: ⚠️ Potential issue | 🟡 Minor

Array.sort() mutates the caller's array.

getThumbnail sorts the input thumbnails array in-place. Every call site (e.g., getGallery.ts) that passes a reference like h.thumbnail?.contents will have its source data reordered as a side effect. Use toSorted() or spread before sorting.

🐛 Proposed fix

 export function getThumbnail(thumbnails: { url: string, width: number }[]): string {
   if (!thumbnails || thumbnails.length === 0) return '';
-  return thumbnails.sort((a, b) => (b.width || 0) - (a.width || 0))[0]?.url || '';
+  return [...thumbnails].sort((a, b) => (b.width || 0) - (a.width || 0))[0]?.url || '';
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/backend/utils.ts` around lines 51 - 54, getThumbnail currently sorts the
incoming thumbnails array in-place causing callers' data to be mutated; modify
getThumbnail to avoid mutating the input by making a non-mutating copy (e.g.,
use thumbnails.toSorted(...) if available or spread into a new array like
[...thumbnails].sort(...)) before selecting the largest width; keep the same
return behavior and references to getThumbnail so callers such as getGallery use
an unmodified thumbnails source.

src/features/List/index.tsx-175-183 (1)

175-183: ⚠️ Potential issue | 🟡 Minor

Sort-order toggle reads config.sortOrder instead of localSortOrder().

Line 178 reads from the reactive config object while localSortOrder() is the signal used elsewhere for rendering (line 176). If config updates are deferred or batched, the toggled value could be stale. Read from localSortOrder() to stay consistent with the signal-driven approach used for localSortBy.

♻️ Suggested fix

           onclick={() => {
-              const newOrder = config.sortOrder === 'asc' ? 'desc' : 'asc';
+              const newOrder = localSortOrder() === 'asc' ? 'desc' : 'asc';
               setConfig('sortOrder', newOrder);
               setLocalSortOrder(newOrder);
               fetchCollection(listStore.id);

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/features/List/index.tsx` around lines 175 - 183, The toggle currently
computes newOrder from config.sortOrder which can be stale; change the toggle to
read the current value from the signal localSortOrder() instead, then call
setConfig('sortOrder', newOrder), setLocalSortOrder(newOrder), and
fetchCollection(listStore.id) as before so rendering and persisted config stay
in sync (update the onclick handler that references config.sortOrder to use
localSortOrder()).

[coderabbitai]

🛠️ Refactor suggestion | 🟠 Major

Extract duplicated item-mapping logic into a shared helper.

The mapping block at lines 97–118 (playlist path) is nearly identical to lines 127–148 (fallback path). Extracting a mapMusicItem(item, albumId, albumName, fallbackAuthor, fallbackAuthorId) helper eliminates the duplication and makes future changes less error-prone.

♻️ Sketch of extracted helper

+function mapMusicItem(
+  item: InstanceType<typeof YTNodes.MusicResponsiveListItem>,
+  albumId: string,
+  albumName: string,
+  fallbackAuthor: string,
+  fallbackAuthorId: string
+) {
+  const videoId = item.id || (item as any).videoId;
+  if (!videoId) return null;
+  const itemAuthor = item.author?.name || (item as any).authors?.[0]?.name || item.artists?.[0]?.name || fallbackAuthor;
+  const itemAuthorId = item.author?.channel_id || (item as any).authors?.[0]?.channel_id || item.artists?.[0]?.channel_id || fallbackAuthorId;
+  return {
+    id: videoId,
+    title: item.title || '',
+    author: itemAuthor,
+    authorId: itemAuthorId,
+    duration: formatDuration(item.duration?.text),
+    albumId,
+    type: 'song' as const,
+    subtext: albumName
+  };
+}

Then both mapping sites become:

-        finalItems = (playlist.contents || []).map((item) => {
-          if (item.is(YTNodes.MusicResponsiveListItem)) {
-            const musicItem = item.as(YTNodes.MusicResponsiveListItem);
-            // ... 15 lines ...
-          }
-          return null;
-        }).filter(...);
+        finalItems = (playlist.contents || [])
+          .filter((item) => item.is(YTNodes.MusicResponsiveListItem))
+          .map((item) => mapMusicItem(item.as(YTNodes.MusicResponsiveListItem), id, name, author, authorId))
+          .filter((item): item is NonNullable<typeof item> => item !== null);

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/backend/getAlbum.ts` around lines 97 - 148, The playlist and fallback
mapping blocks duplicate logic for converting YTNodes.MusicResponsiveListItem
into song objects; extract that into a helper function mapMusicItem(item,
albumId, albumName, fallbackAuthor, fallbackAuthorId) that encapsulates the
checks (item.is(YTNodes.MusicResponsiveListItem)), casts
(item.as(YTNodes.MusicResponsiveListItem)), id resolution (musicItem.id ||
(musicItem as any).videoId), author/authorId fallbacks, duration formatting via
formatDuration, and the returned object shape used to populate finalItems; then
replace both (playlist.contents || []).map(...) and (album.contents ||
[]).map(...) with .map(i => mapMusicItem(i, id, name, author, authorId)). Ensure
the helper returns null for non-matching items so existing .filter(...) logic
stays the same.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Missing null guards — potential NPE on video properties.

Lines 26–29 access v.title.toString(), v.author.name, v.author.id, and v.duration.text without optional chaining. If any playlist video has missing metadata (e.g., a deleted or private video), this will throw at runtime. Compare with streamMapper in src/backend/utils.ts which consistently uses ?. and fallback defaults.

Proposed fix — add safe access with fallbacks

         return {
           id: v.id,
-          title: v.title.toString(),
-          author: v.author.name,
-          authorId: v.author.id,
-          duration: formatDuration(v.duration.text),
+          title: v.title?.toString() || 'Unknown',
+          author: v.author?.name || 'Unknown',
+          authorId: v.author?.id || '',
+          duration: formatDuration(v.duration?.text),
           subtext,
           type: 'video' as const
         };

📝 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.

Suggested change

	return {
	id: v.id,
	title: v.title.toString(),
	author: v.author.name,
	authorId: v.author.id,
	duration: formatDuration(v.duration.text),
	subtext,
	type: 'video' as const
	};
	}
	return {
	id: v.id,
	title: v.title?.toString() || 'Unknown',
	author: v.author?.name || 'Unknown',
	authorId: v.author?.id || '',
	duration: formatDuration(v.duration?.text),
	subtext,
	type: 'video' as const
	};
	}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/backend/getPlaylist.ts` around lines 24 - 33, The mapping of playlist
items can throw NPEs because properties on `v` (title, author, duration) are
accessed directly; update the mapper in getPlaylist (the block returning
id/title/author/authorId/duration/type) to use optional chaining and sensible
fallbacks (e.g., `v.title?.toString() ?? ''`, `v.author?.name ?? 'Unknown'`,
`v.author?.id ?? ''`, `v.duration?.text ?? '0:00'`) similar to `streamMapper` in
`src/backend/utils.ts`, so missing or private/deleted videos won't crash and
will return safe default values.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

cat -n src/backend/getSearch.ts

Repository: n-ce/ytify

Length of output: 1922

---

🏁 Script executed:

rg -n "music\.search" --type=ts -C3

Repository: n-ce/ytify

Length of output: 1088

---

🏁 Script executed:

rg -n "results\.(songs|artists|albums)" --type=ts -C2

Repository: n-ce/ytify

Length of output: 489

---

Shelf selection ignores the actual filter value — may return wrong result type.

Line 10 always picks the first truthy shelf (results.songs || results.artists || results.albums) regardless of which f was requested. If f === 'album' but the API response includes a truthy results.songs, the function returns songs instead of albums.

Map f directly to the corresponding shelf:

Proposed fix

-    const shelf = results.songs || results.artists || results.albums;
+    const shelfMap = { song: results.songs, artist: results.artists, album: results.albums } as const;
+    const shelf = shelfMap[f];

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/backend/getSearch.ts` around lines 8 - 14, The code picks the first
truthy shelf instead of the shelf matching the requested filter (f), causing
wrong result types; update the shelf selection in getSearch (use results.songs
when f === 'song', results.artists when f === 'artist', and results.albums when
f === 'album') and then map/filter that shelf using streamMapper for 'song' and
listMapper for the other types (keep the existing non-null type guard). Ensure
you reference the variables/results objects named results, shelf, and the
mappers streamMapper and listMapper when making the change.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# Search for LASTFM references and env var usage
rg -n 'LASTFM' --type=ts

Repository: n-ce/ytify

Length of output: 36

---

🏁 Script executed:

# Check the actual file content
cat -n src/backend/getSimilar.ts

Repository: n-ce/ytify

Length of output: 1786

---

🏁 Script executed:

# Search for the specific API key mentioned
rg -n '0867bcb6f36c879398969db682a7b69b'

Repository: n-ce/ytify

Length of output: 135

---

🏁 Script executed:

# Check if environment variables are used elsewhere in the codebase
rg -n 'process.env' --type=ts | head -20

Repository: n-ce/ytify

Length of output: 201

---

🏁 Script executed:

# Look for lastfm_api.ts to verify duplicate types
cat -n src/backend/lastfm_api.ts

Repository: n-ce/ytify

Length of output: 111

---

Move hard-coded Last.fm API key to environment variable.

The API key on line 20 is committed directly in source code. Use an environment variable (e.g., process.env.LASTFM_API_KEY) instead. The codebase already uses process.env for configuration in other files.

Additionally, lines 23-24 use async/await syntax; per coding guidelines, prefer Promise-based .then().catch() syntax instead.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/backend/getSimilar.ts` around lines 20 - 21, Replace the hard-coded
Last.fm API key in getSimilar.ts by reading process.env.LASTFM_API_KEY into the
apiKey variable (and add a guard that throws or logs a clear error if it's
missing) instead of the literal '0867bcb6f36c879398969db682a7b69b'; then
refactor the fetch call that uses async/await (around the logic that builds url
and awaits fetch/response.json) to use Promise chaining with .then().catch() so
the network request and JSON parsing use .then(response =>
response.json()).then(data => ...) and a final .catch(err => ...) for error
handling. Ensure you reference/modify the apiKey constant and the function that
performs the fetch in this file.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
echo "=== Checking getStreamData.ts structure ==="
fd "getStreamData.ts" --type=f | head -5 | xargs -I {} wc -l "{}" && echo "" && fd "getStreamData.ts" --type=f | head -1 | xargs cat -n

echo ""
echo "=== References to store.invidious ==="
rg -n --type=ts "store\.invidious" -C2

echo ""
echo "=== References setting invidious in store ==="
rg -n --type=ts "setStore.*invidious" -C2

echo ""
echo "=== Other references to ubiquitous-rugelach ==="
rg -n "ubiquitous-rugelach"

Repository: n-ce/ytify

Length of output: 2270

---

Dead code and breaking change: hardcoded single-entry array breaks fallback logic and store dependency.

The invidious array at line 9 contains only one entry, making the retry logic at line 33 unreachable — when the first instance fails, index + 1 === invidious.length (line 27) immediately triggers the termination condition. More critically, store.invidious is still read by src/lib/utils/player.ts and src/lib/utils/helpers.ts, but the code no longer sets it anywhere, creating a breaking change. If fallback instances are no longer needed, simplify the function; if they are, populate store.invidious dynamically or extend the hardcoded array.

Additionally, remove the async keyword from line 3 — the function uses Promise-based .then().catch() syntax throughout with no await calls, making async unnecessary per coding guidelines.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/lib/modules/getStreamData.ts` at line 9, The invidious constant in
getStreamData and the async annotation are wrong: ensure getStreamData (remove
the unnecessary async keyword) and fix the invidious/fallback logic by either
(A) restoring multiple Invidious instances in the invidious array so the
existing retry logic (the index + 1 === invidious.length check and the .catch
retry path) can work, or (B) if you no longer need fallbacks, simplify
getStreamData to a single-instance flow and explicitly set store.invidious from
that value so other modules (store.invidious referenced by player.ts and
helpers.ts) still see a value; update the code paths that depend on invidious
(the invidious array, getStreamData, and assignments to store.invidious) to be
consistent with the chosen approach.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# First, let's look at the file and confirm the piped declaration
cat -n src/lib/modules/importSongshiftStreams.ts | head -50

Repository: n-ce/ytify

Length of output: 1946

---

🏁 Script executed:

#!/bin/bash
# Check for all usages of piped variable to confirm there are no reassignments
rg -n "piped" src/lib/modules/importSongshiftStreams.ts

Repository: n-ce/ytify

Length of output: 300

---

piped must be declared as an array — currently the SongShift import is non-functional.

const piped = 'https://api.piped.private.coffee' (line 13) is a string. Accessing piped[apiIndex] returns a single character (e.g., 'h' when apiIndex=0), so every fetch call becomes fetch('h/search?...') — an invalid relative URL that will never resolve. The retry guard on line 34 (apiIndex < piped.length - 1) allows up to 43 retry iterations, all with single-character "URLs". No track is ever imported.

-const piped = 'https://api.piped.private.coffee';
+const piped = ['https://api.piped.private.coffee'];

📝 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.

Suggested change

	): Promise<TrackItem & { src: string } | undefined> => await fetch(`${piped[apiIndex]}/search?q=${encodeURIComponent(seed)}&filter=music_songs`)
	const piped = ['https://api.piped.private.coffee'];

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/lib/modules/importSongshiftStreams.ts` at line 18, The variable piped is
currently a string so piped[apiIndex] yields single characters (breaking fetch);
change piped to an array of endpoint strings (e.g., const piped =
['https://api.piped.private.coffee']) and update any logic that assumes array
length (the retry guard using apiIndex < piped.length - 1) to iterate over
actual endpoints; ensure the fetch call in the function that returns
Promise<TrackItem & { src: string } | undefined> uses piped[apiIndex] as a full
URL and any default/apiIndex initialization matches the new array type.

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check if the new keys exist in all locale files
echo "=== player_video_resolution ==="
fd -e json . src/locales --exec grep -l "player_video_resolution" {} \;
echo ""
echo "=== Total locale files ==="
fd -e json . src/locales | wc -l
echo ""
echo "=== queue_reload_notification ==="
fd -e json . src/locales --exec grep -l "queue_reload_notification" {} \;

Repository: n-ce/ytify

Length of output: 733

---

Add missing queue_reload_notification key to 13 locale files.

player_video_resolution is correctly present in all 21 locale files. However, queue_reload_notification is missing from 13 locale files: ur.json, pt.json, ru.json, tr.json, zh.json, pl.json, ro.json, id.json, ja.json, hi.json, es.json, ar.json, and bn.json. This will cause missing translations at runtime for these languages.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/locales/de.json` around lines 223 - 224, Add the missing
"queue_reload_notification" key to the 13 locale files (ur.json, pt.json,
ru.json, tr.json, zh.json, pl.json, ro.json, id.json, ja.json, hi.json, es.json,
ar.json, bn.json) so translations are available at runtime; use the existing
"player_video_resolution" entry as a locator within each file and insert
"queue_reload_notification" with the appropriate translated string (or a
temporary English fallback) in the same JSON object, ensuring punctuation and
JSON formatting match the other entries.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, get an overview of the repository structure
git ls-files | head -30

Repository: n-ce/ytify

Length of output: 659

---

🏁 Script executed:

# Check the types.d.ts file to see the interfaces mentioned
cat -n src/types.d.ts

Repository: n-ce/ytify

Length of output: 3341

---

🏁 Script executed:

# Look for the ListItem component in List/index.tsx to confirm naming collision
grep -n "ListItem" src/features/List/index.tsx | head -20

Repository: n-ce/ytify

Length of output: 145

---

🏁 Script executed:

# Trace how collections are saved and loaded - check library.ts
fd library\\.ts

Repository: n-ce/ytify

Length of output: 112

---

🏁 Script executed:

# Find all usages of Collection type in the codebase
rg "Collection" --glob '*.{ts,tsx}' -n -B2 -A2

Repository: n-ce/ytify

Length of output: 50366

---

🏁 Script executed:

# Check how item.type is used (song vs video discrimination)
rg "type.*===.*['\"]song['\"]|type.*===.*['\"]video['\"]" --glob '*.{ts,tsx}' -n

Repository: n-ce/ytify

Length of output: 487

---

🏁 Script executed:

# Find how img, albumId, subtext fields are used
rg "\\.img\b|\\.albumId\b|\\.subtext\b" --glob '*.{ts,tsx}' -n

Repository: n-ce/ytify

Length of output: 2019

---

🏁 Script executed:

# Check how getCollectionItems is used and if type is ever reassigned
rg "getCollectionItems" --glob '*.{ts,tsx}' -A 5 -B 2

Repository: n-ce/ytify

Length of output: 4974

---

🏁 Script executed:

# Check the addToCollection implementation details to see what fields are stored
cat -n src/lib/utils/library.ts | sed -n '147,190p'

Repository: n-ce/ytify

Length of output: 1458

---

🏁 Script executed:

# Check if tracks in library.ts are filtered when storing
rg "tracks\[.*\] =" src/lib/utils/library.ts -A 2 -B 2

Repository: n-ce/ytify

Length of output: 304

---

🏁 Script executed:

# Check getLocalCollection to see if type is hardcoded
cat -n src/lib/utils/library.ts | sed -n '366,415p'

Repository: n-ce/ytify

Length of output: 1918

---

Collection stores TrackItem — YTItem-specific fields (img, albumId, subtext, type) are lost on save and corrupted on load.

Collection maps IDs to TrackItem (line 70), which lacks img, albumId, subtext, and type. When a YTItem is saved via addToCollection, these fields are silently dropped. On reload in getLocalCollection (lines 396, 404, 411), all items are explicitly hardcoded with type: 'video' as const, destroying the original 'song' vs 'video' distinction. This breaks any downstream logic relying on type discrimination (e.g., playback routing, filtering), and leaves img, albumId, and subtext undefined where used (thumbnail generation, player album context, UI subtext display).

If this is intentional simplification for library storage, document it clearly. Otherwise, widen Collection to store full YTItem or at minimum preserve type, img, and albumId in TrackItem.

Separately: global ListItem interface (types.d.ts, line 32) shares a name with the ListItem component (src/features/List/index.tsx, line 11), creating potential confusion across the codebase.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/types.d.ts` around lines 17 - 30, The Collection type currently maps IDs
to TrackItem which strips YTItem-specific fields; update the type so Collection
stores YTItem (or at minimum extends TrackItem with img, albumId, subtext, and
type) and then modify addToCollection and getLocalCollection to persist and
restore those fields instead of dropping them or hardcoding type: 'video';
ensure deserialization preserves the original type ('song'|'video') and
auxiliary fields; finally, to avoid confusion, rename the global ListItem
interface or the ListItem React component so their names do not collide across
the codebase.

[coderabbitai]

Actionable comments posted: 7

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

src/lib/modules/importSongshiftStreams.ts (1)

49-55: ⚠️ Potential issue | 🔴 Critical

src is dropped in the .map() — all imports land in a collection named "undefined".

The .map() reconstructs track fields but omits src. The type assertion on line 55 (as (TrackItem & { src: string })[]) masks this at compile time. At runtime, track.src is undefined in every reduce iteration, so JavaScript coerces the key to "undefined" and addToCollection("undefined", [...]) is called with every track — ignoring the per-service grouping entirely.

🐛 Proposed fix — Option A (simplest): drop the redundant re-mapping

-      const imports = d.filter(Boolean).map(v => ({
-        id: v?.id,
-        title: v?.title,
-        author: v?.author,
-        duration: v?.duration,
-        authorId: v?.authorId
-      })) as (TrackItem & { src: string })[];
+      const imports = d.filter(Boolean) as (TrackItem & { src: string })[];

🐛 Proposed fix — Option B: include `src` in the existing map

       const imports = d.filter(Boolean).map(v => ({
         id: v?.id,
         title: v?.title,
         author: v?.author,
         duration: v?.duration,
-        authorId: v?.authorId
+        authorId: v?.authorId,
+        src: v?.src
       })) as (TrackItem & { src: string })[];

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/lib/modules/importSongshiftStreams.ts` around lines 49 - 55, The mapped
`imports` array loses the `src` field causing every track to have undefined src
and land in the "undefined" collection; fix by preserving src when building
imports (update the map in importSongshiftStreams.ts to include src: v?.src) or
simply drop the redundant remap and use the filtered array directly (replace the
map with d.filter(Boolean) cast to (TrackItem & { src: string })[]), ensuring
downstream reduce/addToCollection sees a valid track.src.

🧹 Nitpick comments (3)

src/lib/modules/audioErrorHandler.ts (1)

4-7: Add an explicit return type and name the exported function

The function currently has no return type annotation (implicit void) and is an anonymous default export. Both are minor but worth addressing for traceability and type-safety:

♻️ Suggested change

-export default function(
+export default function audioErrorHandler(
   audio: HTMLAudioElement,
   prefetch = ''
-) {
+): void {

As per coding guidelines: "Use fully-typed, generic, and reusable code over quick fixes."

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/lib/modules/audioErrorHandler.ts` around lines 4 - 7, The
default-exported anonymous function in audioErrorHandler.ts lacks a name and an
explicit return type; rename the function (e.g., export default function
handleAudioError) and add a precise return type annotation (likely void or
Promise<void> depending on implementation) on the function signature to improve
traceability and type-safety; update any internal references if needed to use
the new function name while keeping the same parameters (audio:
HTMLAudioElement, prefetch = '') and export behavior.

src/lib/modules/importSongshiftStreams.ts (1)

14-17: async/await in getMetadata is redundant given the .then() chain — coding guideline violation.

The function body is a single .then().then().then().catch() chain; the async keyword and await on line 17 are unnecessary since the chain already returns a Promise. Dropping them aligns with the project's preference for pure .then().catch() style.

♻️ Proposed refactor

   const getMetadata = (
-    seed: string,
-    src = 'songshift',
-  ): Promise<void | TrackItem & { src: string }> => await fetch(`${piped}/search?q=${encodeURIComponent(seed)}&filter=music_songs`)
+    seed: string,
+    src = 'songshift',
+  ): Promise<void | TrackItem & { src: string }> => fetch(`${piped}/search?q=${encodeURIComponent(seed)}&filter=music_songs`)

As per coding guidelines: "Prefer Promise-based .then().catch() syntax over async/await or try...catch blocks."

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/lib/modules/importSongshiftStreams.ts` around lines 14 - 17, The
getMetadata function currently declares async and uses await while returning a
promise chain; remove the async keyword and the await before fetch in
getMetadata so it directly returns the existing .then().then().then().catch()
promise chain (referencing getMetadata and the fetch(`${piped}/search...`) call)
to comply with the project's preference for Promise-based .then().catch() style.

src/lib/stores/player.ts (1)

70-71: proxy will be undefined at runtime (typed string) if instances is ever empty.

instances[Math.floor(Math.random() * instances.length)] returns undefined when instances.length === 0, but TypeScript types it as string. The four hardcoded URLs make this safe today, but the absence of a fallback means a future accidental empty array would silently set proxy to undefined, causing url.replace(origin, undefined) to produce "undefined" URLs in proxyHandler.

🛡️ Defensive fallback

-  proxy: instances[Math.floor(Math.random() * instances.length)]
+  proxy: instances[Math.floor(Math.random() * instances.length)] ?? instances[0] ?? ''

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/lib/stores/player.ts` around lines 70 - 71, The computed proxy value can
become undefined when instances is empty; change the assignment for the proxy
property (the proxy field in the object where isWatching is set) to use a
defensive fallback so it always yields a string (e.g. use instances.length ?
instances[Math.floor(Math.random()*instances.length)] :
'<fallback-url-or-empty-string>'), and ensure the proxy property is typed as
string (not string | undefined); also check proxyHandler usages that call
url.replace(origin, proxy) to remain safe with the chosen fallback.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/features/Player/Video.tsx`:
- Around line 55-58: The captions mapping in Video.tsx currently builds URLs
with playerStore.proxy which is typed string | null | undefined, causing values
like "null/..." or "undefined/..." when proxy is absent; update the captions
construction (the map that produces captions: data.captions.map(...)) to only
prepend playerStore.proxy when it is a non-empty string (e.g., check typeof
playerStore.proxy === 'string' && playerStore.proxy.trim() !== ''), otherwise
leave c.url unchanged (also ensure proper slash joining if proxy is present) so
<track> receives valid caption URLs.
- Around line 126-136: The onerror handler currently calls video.load() then
video.play() synchronously, causing a race and unhandled Promise rejection;
remove the immediate video.play() in the onerror block (in Video.tsx) and either
rely on the existing onloadstart handler (which already calls video.play()) or,
if you must trigger playback here, call video.play() only after the new source
begins loading and handle its Promise with .then().catch() to swallow/log
errors; update references to playerStore.audio.src replacement the same way and
ensure you do not trigger play before the element is in a playable state.

In `@src/lib/modules/audioErrorHandler.ts`:
- Around line 33-34: The call to audio.play() can return a rejected Promise
(e.g., when playback is interrupted by a subsequent audio.load()), so wrap
audio.play() with a Promise-style handler and do not leave it unhandled; change
the code to audio.play().catch(err => { if (err && (err.name === 'AbortError' ||
/interrupted/.test(err.message))) return; /* surface/log real errors */
console.error('audio.play() failed', err); }); ensuring you use .catch() (not
async/await) and reference the existing audio.load() / audio.play() calls so
only genuine failures are logged while expected abort/interruption rejections
are silenced.
- Line 10: The handler constructs const url = new URL(audio.src) before checking
the '&fallback' early-return, which will throw on an empty src and wastes work
in the fallback branch; move the URL construction so it occurs after the
'&fallback' guard (i.e., only when not returning early) and replace any prior
reads of url in the fallback branch to avoid using it; update references to the
url variable in the rest of audioErrorHandler (or the function handling the
audio error) so the URL is only constructed when needed and protected from empty
audio.src.

In `@src/lib/modules/getStreamData.ts`:
- Around line 17-21: The catch handlers in getStreamData (the fetchData(...)
promise chain) return the caught Error instance (e) which lacks an .error
property and the catch parameter is implicitly any; change the catch callback
signatures to accept e: unknown, normalize it to a plain object matching the
declared return type (Record<'error'|'message', string>) and return that object
instead of the raw Error; specifically, in the first .catch(e => ...) and the
inner .catch(() => e) branches, replace use of the raw e with a constructed {
error: 'fetch_failed', message: typeof e === 'string' ? e : e instanceof Error ?
e.message : String(e) } (or similar descriptive error/error key) so callers can
reliably check 'error' in the result and no any is used.

In `@src/lib/modules/start.ts`:
- Around line 8-10: Remove the redundant type assertion on
drawer.lastMainFeature in the conditional that checks params.size: update the
call to setNavStore to pass drawer.lastMainFeature directly (no "as 'search' |
'library'" cast) since drawer.lastMainFeature is already typed as 'search' |
'library'; leave the rest of the call (setNavStore(..., 'state', true))
unchanged.

In `@src/lib/utils/pure.ts`:
- Around line 3-8: The app currently picks a single proxy from the instances
array at startup (instances) and stores it in proxy, so if that host dies
mid-session all requests from proxyHandler fail; update proxyHandler to rotate
through instances on network/fetch failures (try the next URL in instances,
retry a limited number of times) and if all attempts fail propagate a clear
error back to the caller (or emit an event) so the UI can surface a
reload/connection message; alternatively change the selection strategy to pick a
random instance per-request instead of a single startup-set proxy to avoid
session-level lock-in. Ensure you reference and update the instances array, the
proxy selection logic, and the proxyHandler function to implement
retries/rotation and explicit error propagation.

---

Outside diff comments:
In `@src/lib/modules/importSongshiftStreams.ts`:
- Around line 49-55: The mapped `imports` array loses the `src` field causing
every track to have undefined src and land in the "undefined" collection; fix by
preserving src when building imports (update the map in
importSongshiftStreams.ts to include src: v?.src) or simply drop the redundant
remap and use the filtered array directly (replace the map with
d.filter(Boolean) cast to (TrackItem & { src: string })[]), ensuring downstream
reduce/addToCollection sees a valid track.src.

---

Duplicate comments:
In `@src/lib/modules/getStreamData.ts`:
- Line 3: The exported function is declared with the unnecessary async modifier
(export default async function(...)) despite using a Promise chain with
.then()/.catch(); remove the async keyword so the signature is "export default
function(...)" and verify the implementation still returns the existing Promise
chain (no await/try usage remains) — adjust any internal references if needed to
preserve return value and types for getStreamData's callers.

In `@src/types.d.ts`:
- Line 70: The Collection type currently maps to TrackItem which omits
YTItem-specific fields (img, albumId, subtext, type); update the type so those
fields are preserved by either changing Collection to map to a union (TrackItem
| YTItem) or by extending TrackItem to include the optional YTItem fields;
reference the Collection type and either add YTItem to the value union or
augment TrackItem with optional img, albumId, subtext, type properties so
persisted items retain YT-specific data.
- Around line 32-36: The global interface name ListItem conflicts with the
ListItem UI component; rename the type to a distinct identifier (e.g.,
ListItemData or IListItem) in src/types.d.ts (change interface ListItem to the
new name) and update all usages/imports across the codebase (places referencing
ListItem type annotations, props, or generics) to the new name so the UI
component and the type no longer collide; ensure any exported type references
and JSDoc/comments are updated as well.

---

Nitpick comments:
In `@src/lib/modules/audioErrorHandler.ts`:
- Around line 4-7: The default-exported anonymous function in
audioErrorHandler.ts lacks a name and an explicit return type; rename the
function (e.g., export default function handleAudioError) and add a precise
return type annotation (likely void or Promise<void> depending on
implementation) on the function signature to improve traceability and
type-safety; update any internal references if needed to use the new function
name while keeping the same parameters (audio: HTMLAudioElement, prefetch = '')
and export behavior.

In `@src/lib/modules/importSongshiftStreams.ts`:
- Around line 14-17: The getMetadata function currently declares async and uses
await while returning a promise chain; remove the async keyword and the await
before fetch in getMetadata so it directly returns the existing
.then().then().then().catch() promise chain (referencing getMetadata and the
fetch(`${piped}/search...`) call) to comply with the project's preference for
Promise-based .then().catch() style.

In `@src/lib/stores/player.ts`:
- Around line 70-71: The computed proxy value can become undefined when
instances is empty; change the assignment for the proxy property (the proxy
field in the object where isWatching is set) to use a defensive fallback so it
always yields a string (e.g. use instances.length ?
instances[Math.floor(Math.random()*instances.length)] :
'<fallback-url-or-empty-string>'), and ensure the proxy property is typed as
string (not string | undefined); also check proxyHandler usages that call
url.replace(origin, proxy) to remain safe with the chosen fallback.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Null/undefined proxy produces invalid caption track URLs.

playerStore.proxy is typed string | null | undefined. When it is null or undefined, the concatenation playerStore.proxy + c.url evaluates to "null/api/v1/captions/..." or "undefined/api/v1/captions/...". The <track> element will silently fail to load subtitles with no visible error.

🐛 Proposed fix

      captions: data.captions.map(c => ({
        ...c,
-       url: playerStore.proxy + c.url
+       url: playerStore.proxy ? playerStore.proxy + c.url : c.url
      }))

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/features/Player/Video.tsx` around lines 55 - 58, The captions mapping in
Video.tsx currently builds URLs with playerStore.proxy which is typed string |
null | undefined, causing values like "null/..." or "undefined/..." when proxy
is absent; update the captions construction (the map that produces captions:
data.captions.map(...)) to only prepend playerStore.proxy when it is a non-empty
string (e.g., check typeof playerStore.proxy === 'string' &&
playerStore.proxy.trim() !== ''), otherwise leave c.url unchanged (also ensure
proper slash joining if proxy is present) so <track> receives valid caption
URLs.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

new URL(audio.src) is constructed before the &fallback early-return and will throw on an empty src

Two related concerns:

1. new URL('') throws a TypeError synchronously. Although an audio error event with an empty src is unlikely, the constructor is called unconditionally and will crash the handler if it ever fires in that state.
2. In the &fallback branch (lines 12-17), url is read before it is ever used, doing unnecessary work for an early-return path.

Moving the construction to after the &fallback guard eliminates both issues:

🛠️ Proposed fix

   audio.pause();
   const { proxy } = playerStore;
-  const url = new URL(audio.src);

   if (audio.src.endsWith('&fallback')) {
     if (!playerStore.isWatching && !prefetch) {
       setStore('snackbar', 'Error 403 : Unauthenticated Stream');
       setPlayerStore('playbackState', 'none');
     }
     return;
   }

+  const url = new URL(audio.src);
+
   if (!proxy || url.origin === proxy) {

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/lib/modules/audioErrorHandler.ts` at line 10, The handler constructs
const url = new URL(audio.src) before checking the '&fallback' early-return,
which will throw on an empty src and wastes work in the fallback branch; move
the URL construction so it occurs after the '&fallback' guard (i.e., only when
not returning early) and replace any prior reads of url in the fallback branch
to avoid using it; update references to the url variable in the rest of
audioErrorHandler (or the function handling the audio error) so the URL is only
constructed when needed and protected from empty audio.src.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

audio.play() rejection is unhandled — add .catch()

HTMLMediaElement.play() returns a Promise that rejects for any reason playback cannot begin, such as permission issues or interruption. The browser will surface "Uncaught (in promise) DOMException: The play() request was interrupted by a call to pause()" or "…interrupted by a new load request" in DevTools when the rejection goes unhandled. Since audio.load() is called on the line immediately before, pending play promises that are in-flight are immediately resolved or rejected when the media element starts loading a new resource, making an interleaved rejection very likely in practice.

At minimum, silence the expected abort/load-interrupt cases and surface genuine failures:

🛠️ Proposed fix

-  audio.load();
-  audio.play();
+  audio.load();
+  audio.play().catch((err: DOMException) => {
+    if (err.name !== 'AbortError') {
+      console.error('AudioErrorHandler: proxy play failed', err);
+    }
+  });

As per coding guidelines: "Prefer Promise-based .then().catch() syntax over async/await or try...catch blocks."

📝 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.

Suggested change

	audio.load();
	audio.play();
	audio.load();
	audio.play().catch((err: DOMException) => {
	if (err.name !== 'AbortError') {
	console.error('AudioErrorHandler: proxy play failed', err);
	}
	});

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/lib/modules/audioErrorHandler.ts` around lines 33 - 34, The call to
audio.play() can return a rejected Promise (e.g., when playback is interrupted
by a subsequent audio.load()), so wrap audio.play() with a Promise-style handler
and do not leave it unhandled; change the code to audio.play().catch(err => { if
(err && (err.name === 'AbortError' || /interrupted/.test(err.message))) return;
/* surface/log real errors */ console.error('audio.play() failed', err); });
ensuring you use .catch() (not async/await) and reference the existing
audio.load() / audio.play() calls so only genuine failures are logged while
expected abort/interruption rejections are silenced.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Type mismatch: .catch resolves with Error instance, not Record<'error' | 'message', string>.

The declared return type is Promise<Invidious | Record<'error' | 'message', string>>, but both catch branches at lines 19–20 resolve with e — an Error object thrown at line 14 (throw new Error(...)). An Error instance has .message but no .error property, so any caller that checks 'error' in result to distinguish the error path will silently receive false, breaking error detection downstream.

Additionally, Promise.prototype.catch() is not affected by the useUnknownInCatchVariables compiler option, so the catch-callback variable e is always typed any, which violates the guideline to avoid any.

Fix both issues by explicitly typing e as unknown and returning a plain object matching the declared type:

🛡️ Proposed fix

-  return fetchData(playerStore.proxy)
-    .catch(e => {
-      return prefetch ? e : fetchData()
-        .catch(() => e);
-    });
+  const toErrorRecord = (e: unknown): Record<'error' | 'message', string> => {
+    const msg = e instanceof Error ? e.message : String(e);
+    return { error: msg, message: msg };
+  };
+
+  return fetchData(playerStore.proxy)
+    .catch((e: unknown) => {
+      if (prefetch) return toErrorRecord(e);
+      return fetchData().catch((e2: unknown) => toErrorRecord(e2 ?? e));
+    });

As per coding guidelines: "Use fully-typed, generic, and reusable code over quick fixes; avoid the use of any type."

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/lib/modules/getStreamData.ts` around lines 17 - 21, The catch handlers in
getStreamData (the fetchData(...) promise chain) return the caught Error
instance (e) which lacks an .error property and the catch parameter is
implicitly any; change the catch callback signatures to accept e: unknown,
normalize it to a plain object matching the declared return type
(Record<'error'|'message', string>) and return that object instead of the raw
Error; specifically, in the first .catch(e => ...) and the inner .catch(() => e)
branches, replace use of the raw e with a constructed { error: 'fetch_failed',
message: typeof e === 'string' ? e : e instanceof Error ? e.message : String(e)
} (or similar descriptive error/error key) so callers can reliably check 'error'
in the result and no any is used.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

# First, let's find and examine the start.ts file
find . -name "start.ts" -type f | head -20

Repository: n-ce/ytify

Length of output: 80

---

🏁 Script executed:

# Read the start.ts file to examine lines 8-10 and surrounding context
cat -n src/lib/modules/start.ts

echo ""
echo "=== Now finding config.ts ==="
find . -name "config.ts" -type f

Repository: n-ce/ytify

Length of output: 3557

---

🏁 Script executed:

# Read the config.ts file to check the type declaration for drawer.lastMainFeature
cat -n src/lib/utils/config.ts

Repository: n-ce/ytify

Length of output: 2226

---

🏁 Script executed:

# Verify the module loading order by checking where start() is invoked
rg -n "import.*start" --type=ts -A2 -B2

Repository: n-ce/ytify

Length of output: 342

---

Remove redundant type assertion on drawer.lastMainFeature

The as 'search' | 'library' cast on line 9 is redundant: drawer.lastMainFeature is already declared as 'search' | 'library' in src/lib/utils/config.ts (line 51).

♻️ Suggested cleanup

-    setNavStore(drawer.lastMainFeature as 'search' | 'library', 'state', true);
+    setNavStore(drawer.lastMainFeature, 'state', true);

📝 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.

Suggested change

	if (!params.size) {
	setNavStore(drawer.lastMainFeature as 'search' | 'library', 'state', true);
	}
	if (!params.size) {
	setNavStore(drawer.lastMainFeature, 'state', true);
	}

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/lib/modules/start.ts` around lines 8 - 10, Remove the redundant type
assertion on drawer.lastMainFeature in the conditional that checks params.size:
update the call to setNavStore to pass drawer.lastMainFeature directly (no "as
'search' | 'library'" cast) since drawer.lastMainFeature is already typed as
'search' | 'library'; leave the rest of the call (setNavStore(..., 'state',
true)) unchanged.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

No session-level failover if the randomly-selected proxy instance goes offline.

proxy is set once at startup from a random element of instances. If that instance goes down mid-session, all proxied audio stream requests silently fail for Topic-channel tracks — with no rotation or retry until the page is reloaded. Consider rotating to the next instance on proxyHandler failure, or at minimum surface the error to the user so they know to reload.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/lib/utils/pure.ts` around lines 3 - 8, The app currently picks a single
proxy from the instances array at startup (instances) and stores it in proxy, so
if that host dies mid-session all requests from proxyHandler fail; update
proxyHandler to rotate through instances on network/fetch failures (try the next
URL in instances, retry a limited number of times) and if all attempts fail
propagate a clear error back to the caller (or emit an event) so the UI can
surface a reload/connection message; alternatively change the selection strategy
to pick a random instance per-request instead of a single startup-set proxy to
avoid session-level lock-in. Ensure you reference and update the instances
array, the proxy selection logic, and the proxyHandler function to implement
retries/rotation and explicit error propagation.

[coderabbitai]

Actionable comments posted: 13

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)

src/lib/stores/navigation.ts (1)

46-47: ⚠️ Potential issue | 🟡 Minor

Magic index 3 is now off-by-one after inserting library between search and list.

The old navStore layout was queue(0), player(1), home(2), list(3), … so >= 3 correctly excluded the home-view features and only scroll-triggered for list and above. The new layout is queue(0), player(1), search(2), library(3), list(4), …, meaning library now falls inside the scroll-trigger range, while it should behave the same as search.

Update the threshold to >= 4, or — to make this robust against future reordering — derive the boundary dynamically:

🐛 Proposed fix

-  if (removedIndex >= 3)
+  if (removedIndex >= 4)
     closestRef?.scrollIntoView();

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/lib/stores/navigation.ts` around lines 46 - 47, The check using the
hardcoded magic index "3" is now incorrect after inserting "library"; update the
condition in the navigation removal logic that references removedIndex and
closestRef.scrollIntoView() to use a derived boundary instead of the literal 3:
either change the threshold to ">= 4" or, better, compute the boundary from the
nav items (e.g., find the index of the last non-scrollable view like "home" or
the index of "list") and compare removedIndex against that dynamic value so
library behaves like search and future reorderings don't break behavior.

src/features/List/Dropdown.tsx (1)

15-23: ⚠️ Potential issue | 🔴 Critical

Bug: in operator on an array checks indices, not values — subscription state will always be wrong for albums.

getLibraryAlbums() returns Album[]. The in operator on an array tests whether the key is a valid index (0, 1, 2…), not whether any element's id matches. listStore.id (e.g., "MPREb...") is never a numeric index, so isSubscribed will always be false for albums, and saving/removing won't reflect correctly.

Proposed fix

     if (isAlbum) {
       const albums = getLibraryAlbums();
-      setSubscribed(listStore.id in albums);
+      setSubscribed(albums.some(a => a.id === listStore.id));
     } else {

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/features/List/Dropdown.tsx` around lines 15 - 23, The album subscription
check uses the `in` operator on the array returned by `getLibraryAlbums()`,
which tests indices not values, causing `setSubscribed` to be wrong; change the
album branch in the `isAlbum` conditional to test membership by iterating the
albums (e.g., use `some(album => album.id === listStore.id)` or build a Set of
ids) and then call `setSubscribed` with that boolean so `isAlbum`,
`getLibraryAlbums()`, `setSubscribed`, and `listStore.id` are used correctly.

src/lib/stores/search.ts (1)

82-95: ⚠️ Potential issue | 🟠 Major

Search results fetch lacks an AbortController — stale responses can overwrite fresh results.

getSearchSuggestions (line 37) correctly aborts prior requests, but getSearchResults (line 84) fires fetch without a signal. If a user triggers two searches in quick succession, the first (slower) response can resolve after the second and overwrite the correct results.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/lib/stores/search.ts` around lines 82 - 95, getSearchResults is firing
fetch without an AbortController so slow responses can overwrite newer results;
mirror getSearchSuggestions by creating/using an AbortController for
getSearchResults (e.g., a module-scoped or closure-scoped controller variable),
abort any existing controller before starting a new request, pass
controller.signal to fetch, and handle AbortError in the catch (do not show a
snackbar for aborts) while still clearing isLoading in finally; reference
getSearchResults, getSearchSuggestions, setSearchStore and setStore when making
these changes.

🧹 Nitpick comments (10)

src/lib/modules/start.ts (1)

33-42: Consider adding .catch() handlers to fire-and-forget getList calls.

The new getList calls for artist and album (lines 40, 42) follow the existing fire-and-forget pattern for channel/playlist, but none of these handle promise rejections. A failed fetch (network error, invalid ID) would result in an unhandled rejection. Per coding guidelines, prefer .then().catch() — at minimum, attaching a .catch() for user-facing error feedback would improve resilience.

♻️ Example

   else if (artist)
-    getList(artist, 'artist')
+    getList(artist, 'artist').catch(() => {/* handle error */})
   else if (album)
-    getList(album, 'album')
+    getList(album, 'album').catch(() => {/* handle error */})

As per coding guidelines, "Prefer Promise-based .then().catch() syntax over async/await or try...catch blocks".

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/lib/modules/start.ts` around lines 33 - 42, The fire-and-forget calls to
getList (for channel, playlist, artist, album) and the fetchCollection
invocation do not handle rejected promises; update each call
(getList(channel,...), getList(playlist,...), getList(artist,...),
getList(album,...), and fetchCollection(collection || shared,...)) to attach a
.then()/.catch() chain and handle errors in the .catch() (e.g., log the error
via processLogger or console.error and/or surface user-facing feedback) so
promise rejections are not left unhandled.

src/lib/stores/navigation.ts (1)

75-77: params.toString() called twice on line 77 when str already holds the result.

♻️ Proposed simplification

  const str = params.toString();
-  history.replaceState({}, '', location.origin + location.pathname + (str && '?') + params.toString());
+  history.replaceState({}, '', location.origin + location.pathname + (str ? '?' + str : ''));

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/lib/stores/navigation.ts` around lines 75 - 77, The code redundantly
calls params.toString() twice; update the history.replaceState call to reuse the
previously computed str variable and only prepend '?' when str is
non-empty—e.g., call history.replaceState({}, '', location.origin +
location.pathname + (str ? '?' + str : '')); this fixes duplicated work and
makes the intent clearer for the variables str, params, and the replaceState
invocation.

src/backend/utils.ts (1)

78-81: getThumbnail mutates the caller's array via .sort().

Array.prototype.sort() sorts in-place. If any caller passes a reference to a thumbnails array they expect to remain unchanged, this silently reorders it.

Proposed fix — copy before sorting

 export function getThumbnail(thumbnails: { url: string, width: number }[]): string {
   if (!thumbnails || thumbnails.length === 0) return '';
-  return thumbnails.sort((a, b) => (b.width || 0) - (a.width || 0))[0]?.url || '';
+  return [...thumbnails].sort((a, b) => (b.width || 0) - (a.width || 0))[0]?.url || '';
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/backend/utils.ts` around lines 78 - 81, getThumbnail currently calls
thumbnails.sort(), which mutates the input array; to avoid side-effects make a
shallow copy before sorting (e.g., copy the thumbnails array into a new array
and sort that) and then return the url of the largest-width item; ensure you
still handle empty/null thumbnails and use the same return fallback logic as in
getThumbnail.

src/features/List/Dropdown.tsx (1)

12-13: Duplicated isAlbum detection logic.

The identical expression listStore.id.startsWith('MPREb') || listStore.type === 'album' appears in both createEffect (line 13) and subscriptionHandler (line 29). Extract to a shared helper or derived signal to keep it DRY.

Proposed refactor

+  const isAlbum = () => listStore.id.startsWith('MPREb') || listStore.type === 'album';
+
   createEffect(() => {
-    const isAlbum = listStore.id.startsWith('MPREb') || listStore.type === 'album';
-    if (isAlbum) {
+    if (isAlbum()) {
       ...
   });

   function subscriptionHandler() {
-    const isAlbum = listStore.id.startsWith('MPREb') || listStore.type === 'album';
-    if (isAlbum) {
+    if (isAlbum()) {
       ...
   }

Also applies to: 28-29

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/features/List/Dropdown.tsx` around lines 12 - 13, The duplicated
album-detection expression should be extracted into a single reusable symbol and
used by both places: create a shared helper or derived signal (e.g., a function
or createMemo named isAlbum or getIsAlbum) that returns
listStore.id.startsWith('MPREb') || listStore.type === 'album', then replace the
inline expressions in createEffect and subscriptionHandler with that helper so
the logic is defined once and referenced in both locations.

src/backend/getPlaylist.ts (1)

4-6: Prefer .then().catch() over async/await.

The function uses async/await throughout (lines 5–6, 37). Per coding guidelines, prefer Promise-based .then().catch() syntax. The continuation loop (lines 35–39) makes this harder to convert, but the initial fetch chain can be restructured.

As per coding guidelines, **/*.{ts,tsx}: "Prefer Promise-based .then().catch() syntax over async/await or try...catch blocks".

Also applies to: 35-40

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/backend/getPlaylist.ts` around lines 4 - 6, The exported async function
currently uses async/await (calling getClient() and yt.getPlaylist(id)) and a
continuation loop; refactor getPlaylist.ts's default export to use Promise
chaining instead: call getClient().then(yt =>
yt.getPlaylist(id)).then(initialPlaylist => { /* handle initialPlaylist and run
the continuation loop using .then() on subsequent yt.getPlaylist(nextId) or
Promise.resolve chains to accumulate items into the playlist variable */
}).catch(err => { /* handle/log error */ }); Ensure you keep the same
YTPlaylistItem return semantics by resolving a final Promise with the assembled
playlist, reference getClient, yt.getPlaylist, the playlist accumulation
variable, and the continuation loop logic (currently around lines handling
continuation) when converting awaits to .then().catch().

src/components/StreamItem.tsx (1)

58-63: Non-reactive derivations from listStore in component body.

isAlbum (line 58) reads listStore.type outside a reactive context (not in a createMemo, createEffect, or JSX expression). In SolidJS, this value is captured once at render time and won't update if listStore.type changes while the component is mounted. This is likely fine if StreamItem instances are recreated on list changes, but worth noting if the component is ever reused across list transitions.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/components/StreamItem.tsx` around lines 58 - 63, The component reads
listStore.type into the local boolean isAlbum in the StreamItem body, which
captures the signal once and won't react to later changes; make this derivation
reactive by moving the logic into a Solid reactive primitive (e.g., wrap the
expression in createMemo or compute it inside a createEffect/JSX expression) so
isAlbum updates when listStore.type changes; update references to isAlbum (and
any other derived booleans like isFromArtist/isMusic if needed) to use the new
memoized value to ensure the image logic (setImage(generateImageUrl(...))) runs
with up-to-date listStore.type.

src/lib/utils/library.ts (3)

136-138: setDrawer (localStorage write) called once per existing track inside the loop.

libraryPlays is a reference to the same object throughout the loop, so the mutations accumulate correctly — but setDrawer serialises and persists on every hit. Moving the call outside the loop reduces redundant I/O.

♻️ Proposed fix

  for (const item of data) {
    ...
    if (id in tracks) {
      libraryPlays[id] = (libraryPlays[id] || 1) + 1;
-     setDrawer('libraryPlays', libraryPlays);

      if (config.dbsync) { ... }
    } else { ... }
  }

+ setDrawer('libraryPlays', libraryPlays);
  saveCollection(name, collection);

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/lib/utils/library.ts` around lines 136 - 138, The loop is calling
setDrawer('libraryPlays', libraryPlays) for every existing track, causing
redundant localStorage writes; instead, update the in-memory libraryPlays object
inside the loop (e.g., using the existing id and tracks checks) but move the
single call to setDrawer('libraryPlays', libraryPlays) outside/after the loop so
persistence happens once after all mutations are applied.

---

474-474: console.log diagnostic left in production path of cleanseLibraryData.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/lib/utils/library.ts` at line 474, A leftover console.log in
cleanseLibraryData prints diagnostics to stdout in production; replace it with a
proper logger call (e.g., processLogger or the module's logger) or remove it.
Locate the line that uses console.log with the template string referencing key,
collection, and validCollection and either remove the statement or change it to
use the existing logging facility (processLogger.error/info/debug) with the same
message and context so production logging is consistent.

---

425-432: const declaration inside case clause without block braces and duplicate parseDuration logic.

Two issues in the 'duration' case:

1. const inside a case/default clause without wrapping {} is disallowed by no-case-declarations because the binding is visible in the entire switch block but only initialized when that case is reached.
2. parseDuration is already exported from src/lib/stores/queue.ts — importing it avoids the duplication.

♻️ Proposed fix

Import parseDuration at the top of the file:

+import { parseDuration } from '@lib/stores/queue';

Then remove the local definition and wrap the case in a block:

-     case 'duration':
-       const parseDuration = (d: string) => {
-         const parts = d.split(':').map(Number);
-         if (parts.length === 3) return parts[0] * 3600 + parts[1] * 60 + parts[2];
-         if (parts.length === 2) return parts[0] * 60 + parts[1];
-         return 0;
-       };
-       result = parseDuration(a.duration) - parseDuration(b.duration);
-       break;
+     case 'duration': {
+       result = parseDuration(a.duration) - parseDuration(b.duration);
+       break;
+     }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/lib/utils/library.ts` around lines 425 - 432, Remove the duplicated local
parseDuration and instead import the existing parseDuration from
src/lib/stores/queue.ts at the top of the file, then in the switch 'duration'
case delete the local const parseDuration definition and use the imported
parseDuration when computing result; also wrap the 'duration' case body in
braces ({ ... }) to avoid no-case-declarations issues if you later add
block-scoped declarations.

src/lib/stores/list.ts (1)

61-101: async/await combined with try/catch violates the project's coding guideline.

getList uses async/await with a try/catch block. The guideline mandates .then().catch() syntax exclusively.

♻️ Proposed refactor sketch

-export async function getList(
+export function getList(
   id: string,
   type: 'playlist' | 'channel' | 'album' | 'artist',
   all?: boolean
 ) {
   setListStore('hasContinuation', false);
   setListStore('isLoading', true);
   // ... nav setup ...

-  try {
-    const res = await fetch(`${store.api}/api/${type}?id=${id}${all ? '&all=true' : ''}`);
-    if (!res.ok) throw new Error(`Failed to fetch ${type}`);
-    const data = await res.json() as YTListItem;
-    // ... data handling ...
-  } catch (e) {
-    setStore('snackbar', e instanceof Error ? e.message : 'Unknown error');
-    resetList();
-  }
-  setListStore('isLoading', false);
+  fetch(`${store.api}/api/${type}?id=${encodeURIComponent(id)}${all ? '&all=true' : ''}`)
+    .then(res => {
+      if (!res.ok) throw new Error(`Failed to fetch ${type}`);
+      return res.json() as Promise<YTListItem>;
+    })
+    .then(data => {
+      // ... data handling (same logic) ...
+    })
+    .catch(e => {
+      setStore('snackbar', e instanceof Error ? e.message : 'Unknown error');
+      resetList();
+    })
+    .finally(() => setListStore('isLoading', false));
 }

As per coding guidelines, "Prefer Promise-based .then().catch() syntax over async/await or try...catch blocks".

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/lib/stores/list.ts` around lines 61 - 101, The getList implementation
currently uses async/await with try/catch; refactor it to use Promise chains
(.then().catch()) instead so it adheres to the project guideline: replace the
await fetch(...) and subsequent awaits with fetch(...).then(res => { if
(!res.ok) throw new Error(...) ; return res.json() as Promise<YTListItem>
}).then(data => { /* move the artist vs other branching and setListStore(...)
logic here, preserving type assertions and transformations (YTArtistItem,
YTPlaylistItem|YTChannelItem|YTAlbumItem), including author/topic suffixing and
hasContinuation handling) */ }).catch(e => { setStore('snackbar', e instanceof
Error ? e.message : 'Unknown error'); resetList(); }); ensure you reference and
preserve the existing calls to setListStore, setStore('snackbar'), and resetList
and keep the same values/conditionals as in the original code.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/backend/utils.ts`:
- Around line 111-114: The code is using multiple (i: any)/(r: any) casts which
bypass type safety; replace those with typed guards and small reusable helpers:
create a type predicate (e.g., isMenuNavigationItem(node: Helpers.YTNode): node
is YTNodes.MenuNavigationItem) and a helper function like
getPlaylistIdFromMenu(menu?: Helpers.YTNode) that encapsulates the find logic
currently using song.menu?.items?.find(...) and returns a strongly typed
playlistId, and similarly create a helper findRunMatching(subtitle?:
SubtitleType, predicate: (run: SubtitleRunType) => boolean) for the
subtitle?.runs?.find(...) usages; update the call sites (the playlistId
assignment and the places referencing subtitle?.runs?.find and
(item.overlay.content as any).endpoint) to use these helpers and the specific
YTNodes.* subtypes instead of any.
- Around line 5-15: getClient currently races because multiple callers can call
Innertube.create() before youtube is set; change it to store a pending promise
(e.g., youtubePromise) when starting Innertube.create() so concurrent callers
return the same promise, then on resolution set the cached youtube variable;
also ensure that on rejection you clear the pending promise so future calls can
retry. Update references in getClient to check and return youtube first, then
check/assign youtubePromise before calling Innertube.create(), and finally set
youtube = client and clear youtubePromise in the success path (and clear on
failure).
- Around line 169-191: The Channel branch (node.is(YTNodes.Channel)) calls
channel.subscriber_count.toString() and channel.video_count.toString(), which
can throw if those fields are undefined; change these to use optional chaining
and safe fallbacks (e.g., channel.subscriber_count?.toString() ?? "0" and
channel.video_count?.toString() ?? "") so downstream logic and the returned
object (subscribers, videoCount) never throws; update the variables where used
(including the subscribers default assignment and the img construction via
getThumbnailId(channel.author?.thumbnails?.[0]?.url)) to rely on these safe
values.

In `@src/features/List/Dropdown.tsx`:
- Line 167: Remove the debug console.log that prints collectionData on export;
locate the console.log(collectionData) inside the Dropdown component's export
handler (the function invoked when the export button is clicked) and delete it
(or replace it with a conditional debug/logging call that only runs in
non-production environments using an existing logger/ENV check), ensuring no
internal collection data is written to the browser console on export.

In `@src/features/List/List.css`:
- Around line 84-128: The CSS has inconsistent indentation and nesting: move the
"#sortMenu" selector out of the over-indented block under ".sortBar" so it is
nested with two-space indentation (matching other nested rules inside
".sortBar") or if it is intended to be a child of ".sortBar" align it to the
same two-space level as the "i" block; ensure ".loadAllBtn" is a top-level
sibling (no extra indentation) rather than appearing nested under ".sortBar" so
its selector aligns at column 0; update the indentation for "#sortMenu" and
".loadAllBtn" accordingly and verify the closing brace for ".sortBar" is placed
so nesting is correct.

In `@src/features/Search/Search.css`:
- Around line 4-6: The CSS rule for .right-group uses margin-right:
var(--size-000) which resolves to a negative value; update the .right-group
declaration to use a non-negative spacing (e.g., margin-right: var(--size-1) or
margin-right: 0) so the element no longer overflows the header container—locate
the .right-group selector in Search.css and replace var(--size-000) accordingly.

In `@src/lib/modules/libraryMigratorV5.ts`:
- Around line 17-18: OldAlbum declares tracks?: string[] but the album migration
loop in libraryMigratorV5.ts never copies it, causing track lists to be dropped;
either remove tracks from OldAlbum to avoid confusion or update the album
migration code to map the old tracks into the new Album shape. Locate the
OldAlbum type and the album migration block that constructs the new Album
objects, and either delete the tracks property from OldAlbum or add a mapping
like newAlbum.tracks = oldAlbum.tracks (or transform as needed into the new
field name on Album) so track-list data is preserved during migration.
- Around line 76-80: The migration unconditionally sets meta.version = 5 and
writes localStorage.setItem('library_meta', ...) plus shows setStore('snackbar',
'Library updated to V5...Reload') even if any migration section failed; change
the logic in libraryMigratorV5 so that you track overallSuccess (or per-section
flags) across the three migration sections, only set meta.version = 5 and call
localStorage.setItem(...) and setStore('snackbar', ...) after all sections
succeeded, and keep the current try/catch behavior to surface errors without
bumping meta.version so the guard (meta.version !== 4) will allow retries;
alternatively implement per-section completion markers and only bump version
when all markers are present.

In `@src/lib/stores/list.ts`:
- Around line 74-77: The map that builds YTItem objects calls
v.author.endsWith(...) which throws if author is null/undefined; update both
occurrences (the artist list mapping in list: (artist.items || []).map(...) and
the similar albums mapping) to guard or normalize author first — e.g., use a
safe string coerce like const author = (v.author || '') and then set author:
author.endsWith(' - Topic') ? author : `${author} - Topic` (or use
String(v.author || '')). Ensure the resulting value is assigned to the author
field on the YTItem so runtime .endsWith calls never run on null/undefined.
- Line 62: The fetch URL interpolates id raw into the query string which can
break for reserved characters; update the fetch call that uses store.api, type,
id and all so that id is URL-encoded (e.g., use encodeURIComponent(id) or build
the query with URLSearchParams) before interpolation, preserving the all flag
handling, so the request becomes safe for IDs containing &, =, +, etc.

In `@src/lib/stores/navigation.ts`:
- Line 61: The condition treating falsy values as deletion is incorrect: inside
the updateParam function replace the `if (value)` check with an explicit
undefined check (e.g., `value !== undefined`) so that an empty string (`""`)
will be set as a parameter instead of triggering `params.delete(...)`; locate
the branch around `updateParam` that currently calls `params.delete('q')` when
the if fails and change the condition to only delete when `value` is truly
`undefined`.

In `@src/lib/utils/library.ts`:
- Around line 348-352: The code creates items via ids.map(id => tracks[id]) and
passes them to sortCollection causing a TypeError when some entries are
undefined; update the getLocalCollection logic around the ids -> items mapping
(where listStore.reservedCollections, decodeURI(collection), config.sortBy, and
config.sortOrder are used) to filter out falsy/undefined items (e.g., use
.filter(Boolean)) before calling sortCollection and then map to sortedIds,
mirroring the guard already used in getCollectionItems so sortCollection never
receives undefined entries.
- Line 70: getLibraryAlbums returns an array but existing checks treat it like
an object; in src/lib/utils/library.ts replace the object-style check
Object.keys(albums).length > 0 with albums.length > 0 when validating the result
of getLibraryAlbums(), and in src/features/List/Dropdown.tsx replace the
membership check listStore.id in albums with a proper array predicate such as
albums.some(a => a.id === listStore.id) so you test album IDs against the array
returned by getLibraryAlbums().

---

Outside diff comments:
In `@src/features/List/Dropdown.tsx`:
- Around line 15-23: The album subscription check uses the `in` operator on the
array returned by `getLibraryAlbums()`, which tests indices not values, causing
`setSubscribed` to be wrong; change the album branch in the `isAlbum`
conditional to test membership by iterating the albums (e.g., use `some(album =>
album.id === listStore.id)` or build a Set of ids) and then call `setSubscribed`
with that boolean so `isAlbum`, `getLibraryAlbums()`, `setSubscribed`, and
`listStore.id` are used correctly.

In `@src/lib/stores/navigation.ts`:
- Around line 46-47: The check using the hardcoded magic index "3" is now
incorrect after inserting "library"; update the condition in the navigation
removal logic that references removedIndex and closestRef.scrollIntoView() to
use a derived boundary instead of the literal 3: either change the threshold to
">= 4" or, better, compute the boundary from the nav items (e.g., find the index
of the last non-scrollable view like "home" or the index of "list") and compare
removedIndex against that dynamic value so library behaves like search and
future reorderings don't break behavior.

In `@src/lib/stores/search.ts`:
- Around line 82-95: getSearchResults is firing fetch without an AbortController
so slow responses can overwrite newer results; mirror getSearchSuggestions by
creating/using an AbortController for getSearchResults (e.g., a module-scoped or
closure-scoped controller variable), abort any existing controller before
starting a new request, pass controller.signal to fetch, and handle AbortError
in the catch (do not show a snackbar for aborts) while still clearing isLoading
in finally; reference getSearchResults, getSearchSuggestions, setSearchStore and
setStore when making these changes.

---

Duplicate comments:
In `@src/backend/getPlaylist.ts`:
- Line 10: The code in getPlaylist accesses info.author.name and per-video
fields like v.title, v.author.name, v.author.id, and v.duration.text without
null guards; update getPlaylist to use optional chaining and sensible fallbacks
(e.g., const author = info?.author?.name ?? 'Unknown') and when mapping videos
guard each property (e.g., v?.title?.toString() ?? 'Untitled', v?.author?.name
?? 'Unknown', v?.author?.id ?? null, v?.duration?.text ?? '0:00') or skip/filter
out videos missing required metadata so you don't throw on deleted/private
entries; apply these changes to the variables named info and v in the
getPlaylist logic and ensure any downstream consumers handle the fallback
values.

In `@src/features/List/Dropdown.tsx`:
- Around line 124-126: The "View on YouTube" list item is missing an onclick
handler; update the <li> in Dropdown.tsx that renders <i
class="ri-youtube-fill"></i>{listStore.name || t('list_view_on_yt')} to behave
like the other interactive items: attach an onClick that opens the list's
YouTube URL (use listStore.<appropriateUrlField> or build from listStore) in a
new tab/window (or call the shared open/external-link helper used elsewhere),
and add keyboard accessibility (role="button" and onKeyDown handling for
Enter/Space) to match the existing interactive items' behavior.

In `@src/lib/modules/start.ts`:
- Around line 8-10: Remove the redundant type assertion on
drawer.lastMainFeature in the conditional where params.size is falsy; update the
setNavStore call to pass drawer.lastMainFeature directly (no "as 'search' |
'library'") since drawer.lastMainFeature is already typed as 'search' |
'library' (see symbols params.size, setNavStore, drawer.lastMainFeature).

In `@src/lib/utils/library.ts`:
- Line 364: The code unconditionally sets collection items to type: 'video' in
the setListStore call (and the other occurrences at the same review),
overwriting any preserved type on stored songs; change the mapping so it
preserves an existing tracks[id].type when present and only falls back to
'video' otherwise (i.e., use tracks[id].type if defined, otherwise default to
'video'), and apply the same fix to the other occurrences flagged (the mappings
at the other two lines) to avoid overwriting stored item types.

In `@src/types.d.ts`:
- Around line 16-30: The Collection is currently typed to store TrackItem which
lacks img/albumId/subtext/type so saving a YTItem drops those fields; fix by
changing the collection’s stored type to include the extended fields (e.g., use
YTItem or a union TrackItem|YTItem) where Collection is instantiated, or
alternatively extend TrackItem to add the optional img/albumId/subtext/type
fields, and update any save/load/serialize logic in the Collection
implementation (save, load, deserialize/serialize) to use the generic type so
those extra properties are preserved across persistence.

---

Nitpick comments:
In `@src/backend/getPlaylist.ts`:
- Around line 4-6: The exported async function currently uses async/await
(calling getClient() and yt.getPlaylist(id)) and a continuation loop; refactor
getPlaylist.ts's default export to use Promise chaining instead: call
getClient().then(yt => yt.getPlaylist(id)).then(initialPlaylist => { /* handle
initialPlaylist and run the continuation loop using .then() on subsequent
yt.getPlaylist(nextId) or Promise.resolve chains to accumulate items into the
playlist variable */ }).catch(err => { /* handle/log error */ }); Ensure you
keep the same YTPlaylistItem return semantics by resolving a final Promise with
the assembled playlist, reference getClient, yt.getPlaylist, the playlist
accumulation variable, and the continuation loop logic (currently around lines
handling continuation) when converting awaits to .then().catch().

In `@src/backend/utils.ts`:
- Around line 78-81: getThumbnail currently calls thumbnails.sort(), which
mutates the input array; to avoid side-effects make a shallow copy before
sorting (e.g., copy the thumbnails array into a new array and sort that) and
then return the url of the largest-width item; ensure you still handle
empty/null thumbnails and use the same return fallback logic as in getThumbnail.

In `@src/components/StreamItem.tsx`:
- Around line 58-63: The component reads listStore.type into the local boolean
isAlbum in the StreamItem body, which captures the signal once and won't react
to later changes; make this derivation reactive by moving the logic into a Solid
reactive primitive (e.g., wrap the expression in createMemo or compute it inside
a createEffect/JSX expression) so isAlbum updates when listStore.type changes;
update references to isAlbum (and any other derived booleans like
isFromArtist/isMusic if needed) to use the new memoized value to ensure the
image logic (setImage(generateImageUrl(...))) runs with up-to-date
listStore.type.

In `@src/features/List/Dropdown.tsx`:
- Around line 12-13: The duplicated album-detection expression should be
extracted into a single reusable symbol and used by both places: create a shared
helper or derived signal (e.g., a function or createMemo named isAlbum or
getIsAlbum) that returns listStore.id.startsWith('MPREb') || listStore.type ===
'album', then replace the inline expressions in createEffect and
subscriptionHandler with that helper so the logic is defined once and referenced
in both locations.

In `@src/lib/modules/start.ts`:
- Around line 33-42: The fire-and-forget calls to getList (for channel,
playlist, artist, album) and the fetchCollection invocation do not handle
rejected promises; update each call (getList(channel,...),
getList(playlist,...), getList(artist,...), getList(album,...), and
fetchCollection(collection || shared,...)) to attach a .then()/.catch() chain
and handle errors in the .catch() (e.g., log the error via processLogger or
console.error and/or surface user-facing feedback) so promise rejections are not
left unhandled.

In `@src/lib/stores/list.ts`:
- Around line 61-101: The getList implementation currently uses async/await with
try/catch; refactor it to use Promise chains (.then().catch()) instead so it
adheres to the project guideline: replace the await fetch(...) and subsequent
awaits with fetch(...).then(res => { if (!res.ok) throw new Error(...) ; return
res.json() as Promise<YTListItem> }).then(data => { /* move the artist vs other
branching and setListStore(...) logic here, preserving type assertions and
transformations (YTArtistItem, YTPlaylistItem|YTChannelItem|YTAlbumItem),
including author/topic suffixing and hasContinuation handling) */ }).catch(e =>
{ setStore('snackbar', e instanceof Error ? e.message : 'Unknown error');
resetList(); }); ensure you reference and preserve the existing calls to
setListStore, setStore('snackbar'), and resetList and keep the same
values/conditionals as in the original code.

In `@src/lib/stores/navigation.ts`:
- Around line 75-77: The code redundantly calls params.toString() twice; update
the history.replaceState call to reuse the previously computed str variable and
only prepend '?' when str is non-empty—e.g., call history.replaceState({}, '',
location.origin + location.pathname + (str ? '?' + str : '')); this fixes
duplicated work and makes the intent clearer for the variables str, params, and
the replaceState invocation.

In `@src/lib/utils/library.ts`:
- Around line 136-138: The loop is calling setDrawer('libraryPlays',
libraryPlays) for every existing track, causing redundant localStorage writes;
instead, update the in-memory libraryPlays object inside the loop (e.g., using
the existing id and tracks checks) but move the single call to
setDrawer('libraryPlays', libraryPlays) outside/after the loop so persistence
happens once after all mutations are applied.
- Line 474: A leftover console.log in cleanseLibraryData prints diagnostics to
stdout in production; replace it with a proper logger call (e.g., processLogger
or the module's logger) or remove it. Locate the line that uses console.log with
the template string referencing key, collection, and validCollection and either
remove the statement or change it to use the existing logging facility
(processLogger.error/info/debug) with the same message and context so production
logging is consistent.
- Around line 425-432: Remove the duplicated local parseDuration and instead
import the existing parseDuration from src/lib/stores/queue.ts at the top of the
file, then in the switch 'duration' case delete the local const parseDuration
definition and use the imported parseDuration when computing result; also wrap
the 'duration' case body in braces ({ ... }) to avoid no-case-declarations
issues if you later add block-scoped declarations.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Race condition in getClient() — concurrent calls can create duplicate clients.

If two callers invoke getClient() simultaneously before youtube is set, both pass the if (youtube) guard and both call Innertube.create(). Only the last to resolve will be cached. Consider storing the pending promise itself to deduplicate.

Proposed fix

-let youtube: Innertube | null = null;
+let youtube: Innertube | null = null;
+let clientPromise: Promise<Innertube> | null = null;

 export async function getClient(): Promise<Innertube> {
   if (youtube) return Promise.resolve(youtube);
+  if (clientPromise) return clientPromise;

-  return Innertube.create({
+  clientPromise = Innertube.create({
     cache: new UniversalCache(false),
     generate_session_locally: true
   }).then((client) => {
     youtube = client;
+    clientPromise = null;
     return youtube;
   });
+  return clientPromise;
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/backend/utils.ts` around lines 5 - 15, getClient currently races because
multiple callers can call Innertube.create() before youtube is set; change it to
store a pending promise (e.g., youtubePromise) when starting Innertube.create()
so concurrent callers return the same promise, then on resolution set the cached
youtube variable; also ensure that on rejection you clear the pending promise so
future calls can retry. Update references in getClient to check and return
youtube first, then check/assign youtubePromise before calling
Innertube.create(), and finally set youtube = client and clear youtubePromise in
the success path (and clear on failure).

[coderabbitai]

🛠️ Refactor suggestion | 🟠 Major

Multiple any casts undermine type safety.

• Line 111: (i: any) in menu?.items?.find
• Line 199: (r: any) in subtitle?.runs?.find
• Line 211: (item.overlay.content as any).endpoint
• Line 212: (i: any) in menu?.items?.find
• Line 216: (r: any) in subtitle?.runs?.find
• Line 224: (r: any) in subtitle?.runs?.find

These bypass the type checker and prevent catching property-access errors at compile time. Consider narrowing with Helpers.YTNode subtypes or at minimum extracting a typed helper for the repeated menu/subtitle run patterns.

As per coding guidelines, **/*.{ts,tsx}: "Use fully-typed, generic, and reusable code over quick fixes; avoid the use of any type".

Also applies to: 199-199, 211-212, 216-216, 224-224

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/backend/utils.ts` around lines 111 - 114, The code is using multiple (i:
any)/(r: any) casts which bypass type safety; replace those with typed guards
and small reusable helpers: create a type predicate (e.g.,
isMenuNavigationItem(node: Helpers.YTNode): node is YTNodes.MenuNavigationItem)
and a helper function like getPlaylistIdFromMenu(menu?: Helpers.YTNode) that
encapsulates the find logic currently using song.menu?.items?.find(...) and
returns a strongly typed playlistId, and similarly create a helper
findRunMatching(subtitle?: SubtitleType, predicate: (run: SubtitleRunType) =>
boolean) for the subtitle?.runs?.find(...) usages; update the call sites (the
playlistId assignment and the places referencing subtitle?.runs?.find and
(item.overlay.content as any).endpoint) to use these helpers and the specific
YTNodes.* subtypes instead of any.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, let's check the file and understand the context
cat -n src/backend/utils.ts | head -200 | tail -50

Repository: n-ce/ytify

Length of output: 1959

---

🏁 Script executed:

# Look for type definitions or package.json to understand youtubei.js version
find . -name "package.json" -type f -exec grep -l "youtubei" {} \;

Repository: n-ce/ytify

Length of output: 68

---

🏁 Script executed:

# Check if there are any type definitions in the project
find . -name "*.d.ts" -type f | head -20

Repository: n-ce/ytify

Length of output: 90

---

🏁 Script executed:

# Search for any imports or type declarations related to YTNodes.Channel
rg "YTNodes\.Channel|Channel.*from.*youtubei" --type ts --type tsx -A 3

Repository: n-ce/ytify

Length of output: 81

---

🌐 Web query:

youtubei.js Channel node subscriber_count video_count properties always present

💡 Result:

No.

In youtubei.js, the Channel object’s header itself is optional, and when present it can be one of several header types (C4TabbedHeader | CarouselHeader | InteractiveTabbedHeader | PageHeader)—so you cannot assume any specific header fields exist. [1]

If the header is a C4TabbedHeader, the fields that correspond to what you’re calling “subscriber_count” and “video_count” are subscribers? and videos_count?—and both are explicitly optional (they may be missing/undefined). [2]

If you need stable numeric counts, use the YouTube Data API channels.list statistics part (subscriberCount, videoCount)—but note hiddenSubscriberCount can indicate the subscriber count is not publicly visible, and counts can be rounded / only reflect public videos. [3]

Sources: [1] [2] [3]

---

🏁 Script executed:

# Check the type definitions file
cat -n src/types.d.ts

Repository: n-ce/ytify

Length of output: 3319

---

🏁 Script executed:

# Look for YTNodes import/type definition
rg "YTNodes|from.*youtubei" --type ts -B 2 -A 5

Repository: n-ce/ytify

Length of output: 21534

---

🏁 Script executed:

# Check package.json for youtubei.js version
cat package.json | grep -A 2 -B 2 "youtubei"

Repository: n-ce/ytify

Length of output: 177

---

Use optional chaining on channel.subscriber_count and channel.video_count to prevent runtime errors.

Lines 174–175 call .toString() on properties that are explicitly optional in youtubei.js, while the Playlist handler immediately above (line 163) correctly uses optional chaining. If the YouTube API returns a channel without these fields, this will throw a TypeError.

Proposed fix

-    let subscribers = channel.subscriber_count.toString();
-    let videoCount = channel.video_count.toString();
+    let subscribers = channel.subscriber_count?.toString() || '';
+    let videoCount = channel.video_count?.toString() || '';

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/backend/utils.ts` around lines 169 - 191, The Channel branch
(node.is(YTNodes.Channel)) calls channel.subscriber_count.toString() and
channel.video_count.toString(), which can throw if those fields are undefined;
change these to use optional chaining and safe fallbacks (e.g.,
channel.subscriber_count?.toString() ?? "0" and channel.video_count?.toString()
?? "") so downstream logic and the returned object (subscribers, videoCount)
never throws; update the variables where used (including the subscribers default
assignment and the img construction via
getThumbnailId(channel.author?.thumbnails?.[0]?.url)) to rely on these safe
values.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Remove debug console.log from production code.

console.log(collectionData) fires every time the export button is clicked and leaks internal collection data to the console.

Proposed fix

-            console.log(collectionData);

📝 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.

Suggested change

console.log(collectionData);

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/features/List/Dropdown.tsx` at line 167, Remove the debug console.log
that prints collectionData on export; locate the console.log(collectionData)
inside the Dropdown component's export handler (the function invoked when the
export button is clicked) and delete it (or replace it with a conditional
debug/logging call that only runs in non-production environments using an
existing logger/ENV check), ensuring no internal collection data is written to
the browser console on export.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

# First, check the actual state of the CSS file around lines 84-128
cd src/features/List && wc -l List.css && echo "---" && sed -n '80,135p' List.css | cat -n

Repository: n-ce/ytify

Length of output: 1273

---

🏁 Script executed:

# Find the TSX/JSX component file for List
fd -e tsx -e jsx -p '*List*' src/features/List/

Repository: n-ce/ytify

Length of output: 339

---

🏁 Script executed:

# Fix the fd command to find List component files
fd -e tsx -e jsx 'List' src/features/List/

Repository: n-ce/ytify

Length of output: 36

---

🏁 Script executed:

# List all files in the List directory
ls -la src/features/List/

# Also search for any tsx files there
fd -e tsx -e jsx . src/features/List/

Repository: n-ce/ytify

Length of output: 494

---

🏁 Script executed:

# Search for sortBar and loadAllBtn in the component files
rg -n 'sortBar|loadAllBtn' src/features/List/*.tsx -C 3

Repository: n-ce/ytify

Length of output: 1066

---

🏁 Script executed:

# Get more context around sortBar in index.tsx
sed -n '150,200p' src/features/List/index.tsx | cat -n

# Also check where Results component is used in the component hierarchy
rg -n 'Results|sortBar' src/features/List/index.tsx | head -20

Repository: n-ce/ytify

Length of output: 2466

---

🏁 Script executed:

# Check context around Results component usage at line 209
sed -n '205,215p' src/features/List/index.tsx | cat -n

# Check indentation patterns in CSS file (look at lines before sortBar)
sed -n '1,84p' src/features/List/List.css | tail -20 | cat -n

Repository: n-ce/ytify

Length of output: 1073

---

🏁 Script executed:

# Final verification: count braces to confirm .loadAllBtn is inside .sortBar scope in CSS
sed -n '84,128p' src/features/List/List.css | grep -n '{' | head -20
sed -n '84,128p' src/features/List/List.css | grep -n '}' | head -20

Repository: n-ce/ytify

Length of output: 230

---

Fix inconsistent indentation: #sortMenu has 6 spaces instead of 2, and .loadAllBtn is a top-level selector that should have 0-space indentation.

Lines 105–107 show #sortMenu with 6-space indentation instead of the 2-space convention used for nested selectors elsewhere. Additionally, .loadAllBtn (line 110) is a sibling selector to .sortBar, not nested within it, but the 2-space indentation makes this visually unclear. Align both to the proper nesting levels.

Proposed indentation fix

-      `#sortMenu` {
-      margin-right: auto;
-    }
+    `#sortMenu` {
+      margin-right: auto;
+    }
   }
   
-  .loadAllBtn {
+.loadAllBtn {

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/features/List/List.css` around lines 84 - 128, The CSS has inconsistent
indentation and nesting: move the "#sortMenu" selector out of the over-indented
block under ".sortBar" so it is nested with two-space indentation (matching
other nested rules inside ".sortBar") or if it is intended to be a child of
".sortBar" align it to the same two-space level as the "i" block; ensure
".loadAllBtn" is a top-level sibling (no extra indentation) rather than
appearing nested under ".sortBar" so its selector aligns at column 0; update the
indentation for "#sortMenu" and ".loadAllBtn" accordingly and verify the closing
brace for ".sortBar" is placed so nesting is correct.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

id is not URL-encoded in the fetch query string.

id is interpolated raw into the URL. IDs containing &, =, +, or other reserved characters (possible with certain playlist or channel IDs) would produce a malformed request.

🛡️ Proposed fix

- const res = await fetch(`${store.api}/api/${type}?id=${id}${all ? '&all=true' : ''}`);
+ const res = await fetch(`${store.api}/api/${type}?id=${encodeURIComponent(id)}${all ? '&all=true' : ''}`);

📝 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.

Suggested change

	const res = await fetch(`${store.api}/api/${type}?id=${id}${all ? '&all=true' : ''}`);
	const res = await fetch(`${store.api}/api/${type}?id=${encodeURIComponent(id)}${all ? '&all=true' : ''}`);

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/lib/stores/list.ts` at line 62, The fetch URL interpolates id raw into
the query string which can break for reserved characters; update the fetch call
that uses store.api, type, id and all so that id is URL-encoded (e.g., use
encodeURIComponent(id) or build the query with URLSearchParams) before
interpolation, preserving the all flag handling, so the request becomes safe for
IDs containing &, =, +, etc.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

v.author.endsWith() throws TypeError if author is null or undefined.

Both Lines 76 and 94 call v.author.endsWith(...) directly. If the API returns an item without an author field, this throws at runtime and crashes the list render for artists and albums. The same pattern exists at Line 94.

🛡️ Proposed fix (Line 76)

-   author: v.author.endsWith(' - Topic') ? v.author : `${v.author} - Topic`
+   author: v.author ? (v.author.endsWith(' - Topic') ? v.author : `${v.author} - Topic`) : ''

🛡️ Proposed fix (Line 94)

-   author: (data.type === 'album' && !v.author.endsWith(' - Topic')) ? `${v.author} - Topic` : v.author
+   author: (data.type === 'album' && v.author && !v.author.endsWith(' - Topic')) ? `${v.author} - Topic` : (v.author || '')

📝 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.

Suggested change

	list: (artist.items || []).map(v => ({
	...v,
	author: v.author.endsWith(' - Topic') ? v.author : `${v.author} - Topic`
	}) as YTItem),
	list: (artist.items || []).map(v => ({
	...v,
	author: v.author ? (v.author.endsWith(' - Topic') ? v.author : `${v.author} - Topic`) : ''
	}) as YTItem),

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/lib/stores/list.ts` around lines 74 - 77, The map that builds YTItem
objects calls v.author.endsWith(...) which throws if author is null/undefined;
update both occurrences (the artist list mapping in list: (artist.items ||
[]).map(...) and the similar albums mapping) to guard or normalize author first
— e.g., use a safe string coerce like const author = (v.author || '') and then
set author: author.endsWith(' - Topic') ? author : `${author} - Topic` (or use
String(v.author || '')). Ensure the resulting value is assigned to the author
field on the YTItem so runtime .endsWith calls never run on null/undefined.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

if (value) silently treats an empty-string argument as "delete".

value?: string makes "" falsy, so updateParam('q', '') falls through to params.delete('q') instead of setting the param. Prefer an explicit undefined check to make the intent unambiguous:

🐛 Proposed fix

-  if (value) {
+  if (value !== undefined) {

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/lib/stores/navigation.ts` at line 61, The condition treating falsy values
as deletion is incorrect: inside the updateParam function replace the `if
(value)` check with an explicit undefined check (e.g., `value !== undefined`) so
that an empty string (`""`) will be set as a parameter instead of triggering
`params.delete(...)`; locate the branch around `updateParam` that currently
calls `params.delete('q')` when the if fails and change the condition to only
delete when `value` is truly `undefined`.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Search for getLibraryAlbums usages and related patterns in Library components
rg -n "getLibraryAlbums\|\.thumbnail\|Object\.keys.*Albums\|album\.img" src/features/Home/Library/ -A 2 -B 1

Repository: n-ce/ytify

Length of output: 120

---

🏁 Script executed:

#!/bin/bash
# Find Library/index.tsx and related files
fd -t f "Library" --type directory
find . -name "index.tsx" | grep -i library
find . -name "library.ts" -type f
git ls-files | grep -i library | head -20

Repository: n-ce/ytify

Length of output: 559

---

🏁 Script executed:

#!/bin/bash
# First, check the library.ts file to see the getLibraryAlbums function
echo "=== src/lib/utils/library.ts (around line 70) ==="
sed -n '65,75p' src/lib/utils/library.ts

# Check Library/index.tsx
echo -e "\n=== src/features/Library/index.tsx ==="
cat -n src/features/Library/index.tsx

# Search for getLibraryAlbums usage
echo -e "\n=== Search for getLibraryAlbums usages ==="
rg "getLibraryAlbums" --type ts --type tsx -B 2 -A 3

Repository: n-ce/ytify

Length of output: 5854

---

🏁 Script executed:

#!/bin/bash
# Search for getLibraryAlbums usages without file type restriction
echo "=== All getLibraryAlbums usages ==="
rg "getLibraryAlbums" -B 2 -A 3

# Check Gallery.tsx and other Library components
echo -e "\n=== Gallery.tsx ==="
head -100 src/features/Library/Gallery.tsx

echo -e "\n=== Collections.tsx ==="
head -100 src/features/Library/Collections.tsx

echo -e "\n=== Check for Object.keys with Albums ==="
rg "Object\.keys.*[Aa]lbum" -B 2 -A 2

Repository: n-ce/ytify

Length of output: 11824

---

🏁 Script executed:

#!/bin/bash
# Get context for Object.keys(albums) bug in library.ts
echo "=== library.ts: Object.keys(albums) issue ==="
rg -n "Object\.keys.*albums" src/lib/utils/library.ts -B 5 -A 3

# Get context for 'in albums' bug in List/Dropdown.tsx
echo -e "\n=== List/Dropdown.tsx: 'in albums' issue ==="
rg -n "in albums" src/features/List/Dropdown.tsx -B 5 -A 3

# Check the complete functions
echo -e "\n=== Complete context from library.ts ==="
sed -n '170,185p' src/lib/utils/library.ts

echo -e "\n=== Check List/Dropdown.tsx section ==="
rg -n "getLibraryAlbums\|setSubscribed" src/features/List/Dropdown.tsx -B 3 -A 8

Repository: n-ce/ytify

Length of output: 1333

---

Fix type mismatch bugs in library.ts and List/Dropdown.tsx where getLibraryAlbums() is treated as an object instead of an array.

Library/index.tsx is already correctly implemented and doesn't have issues. However, two other files do:

1. src/lib/utils/library.ts line 38: Object.keys(albums).length > 0 treats the array as an object. Change to albums.length > 0.

2. src/features/List/Dropdown.tsx line 17: listStore.id in albums uses the object property check operator on an array. Change to albums.some(a => a.id === listStore.id) to properly check if the album is saved.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/lib/utils/library.ts` at line 70, getLibraryAlbums returns an array but
existing checks treat it like an object; in src/lib/utils/library.ts replace the
object-style check Object.keys(albums).length > 0 with albums.length > 0 when
validating the result of getLibraryAlbums(), and in
src/features/List/Dropdown.tsx replace the membership check listStore.id in
albums with a proper array predicate such as albums.some(a => a.id ===
listStore.id) so you test album IDs against the array returned by
getLibraryAlbums().

[coderabbitai]

⚠️ Potential issue | 🟠 Major

undefined values from orphaned track IDs crash sortCollection.

ids.map(id => tracks[id]) produces undefined for any ID present in the collection but absent from the tracks map. These values flow directly into sortCollection, which accesses a.title, a.author, and a.duration without null guards — causing a runtime TypeError for any non-'modified' sort mode.

getCollectionItems at Line 75 already guards against this with .filter(Boolean); getLocalCollection should do the same.

🛡️ Proposed fix

- const items = ids.map(id => tracks[id]);
+ const items = ids.map(id => tracks[id]).filter(Boolean) as TrackItem[];

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/lib/utils/library.ts` around lines 348 - 352, The code creates items via
ids.map(id => tracks[id]) and passes them to sortCollection causing a TypeError
when some entries are undefined; update the getLocalCollection logic around the
ids -> items mapping (where listStore.reservedCollections,
decodeURI(collection), config.sortBy, and config.sortOrder are used) to filter
out falsy/undefined items (e.g., use .filter(Boolean)) before calling
sortCollection and then map to sortedIds, mirroring the guard already used in
getCollectionItems so sortCollection never receives undefined entries.

[coderabbitai]

Actionable comments posted: 5

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)

netlify/edge-functions/syncHash.ts (3)

21-21: ⚠️ Potential issue | 🟠 Major

Email inserted into validator URL without encodeURIComponent.

User-supplied email values containing +, &, #, or spaces will corrupt the query string or inject additional query parameters, potentially bypassing validation entirely.

🛡️ Proposed fix

-  const validatorUrl = `https://rapid-email-verifier.fly.dev/api/validate?email=${email}`;
+  const validatorUrl = `https://rapid-email-verifier.fly.dev/api/validate?email=${encodeURIComponent(email)}`;

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@netlify/edge-functions/syncHash.ts` at line 21, The validator URL is
constructed without encoding user input which allows characters like "+", "&",
"#" or spaces in the email to break the query string; update the construction of
validatorUrl (in syncHash.ts where validatorUrl is defined) to URL-encode the
email using encodeURIComponent(email) before interpolation so the query
parameter is safe and cannot inject extra params or corrupt the URL.

---

11-11: ⚠️ Potential issue | 🟠 Major

Unhandled req.json() rejection crashes the edge function.

req.json() rejects if the body is not valid JSON or the Content-Type is wrong. Since it is not wrapped in a try/catch (or .catch()), a malformed request will bubble up as an unhandled 500 instead of returning a well-formed 400 Bad Request.

🛡️ Proposed fix

-  const { email, password } = await req.json();
+  const body = await req.json().catch(() => null);
+  if (!body) {
+    return new Response("Invalid JSON body", {
+      status: 400,
+      headers: { "content-type": "text/plain" },
+    });
+  }
+  const { email, password } = body;

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@netlify/edge-functions/syncHash.ts` at line 11, The call to req.json() in
syncHash is unprotected and can throw on invalid JSON; wrap the await req.json()
in a try/catch inside the syncHash handler (or the function that contains the
line "const { email, password } = await req.json();"), catch JSON parse errors
and return a 400 Bad Request response with a clear error message, and only
destructure email/password after validating the parsed body; ensure other errors
still propagate or return 500 as appropriate.

---

48-52: ⚠️ Potential issue | 🟠 Major

SHA-256 is not a password hashing function.

SHA-256 is a fast, general-purpose digest and is trivially brute-forced with GPU rainbow tables. The Web Crypto API (available in Netlify Edge Functions) natively supports PBKDF2, which is the correct primitive for deriving a key/hash from a password.

🔐 Proposed replacement using PBKDF2

-  const msgBuffer = new TextEncoder().encode(combinedString);
-  const hashBuffer = await crypto.subtle.digest('SHA-256', msgBuffer);
-  const hashedPassword = Array
-    .from(new Uint8Array(hashBuffer))
-    .map(b => b.toString(16).padStart(2, '0'))
-    .join('');
+  const keyMaterial = await crypto.subtle.importKey(
+    'raw',
+    new TextEncoder().encode(password),
+    'PBKDF2',
+    false,
+    ['deriveBits']
+  );
+  const hashBuffer = await crypto.subtle.deriveBits(
+    {
+      name: 'PBKDF2',
+      salt: new TextEncoder().encode(normalizedEmail),
+      iterations: 200_000,
+      hash: 'SHA-256',
+    },
+    keyMaterial,
+    256
+  );
+  const hashedPassword = Array
+    .from(new Uint8Array(hashBuffer))
+    .map(b => b.toString(16).padStart(2, '0'))
+    .join('');

Note: Switching to PBKDF2 is also a breaking change for any stored hashes — coordinate this alongside the migration strategy for the separator change above.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@netlify/edge-functions/syncHash.ts` around lines 48 - 52, The code in
syncHash.ts currently uses crypto.subtle.digest('SHA-256') (the const hashBuffer
calculation) which is inappropriate for password hashing; replace that block
with a PBKDF2 derivation: generate or accept a per-user salt, define a constant
iterations count, import the password as a raw key via crypto.subtle.importKey,
then call crypto.subtle.deriveBits with { name: 'PBKDF2', salt, iterations,
hash: 'SHA-256' } to derive the hash bytes and convert to hex; update storage to
persist the salt and iterations (and handle the separator/migration for existing
SHA-256 hashes). Ensure you update any functions referencing hashedPassword/sync
logic to expect PBKDF2 output and include salt+iterations metadata for
verification.

🧹 Nitpick comments (11)

src/features/Search/index.tsx (2)

10-10: Consider naming the default-exported component.

export default function() produces an anonymous component, making it harder to identify in SolidJS DevTools and stack traces.

♻️ Proposed refactor

-export default function() {
+export default function Search() {

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/features/Search/index.tsx` at line 10, The default-exported component is
anonymous (export default function()) which hinders identification in SolidJS
DevTools and stack traces; rename the component (e.g., give the function a
meaningful identifier like Search) and export that named function (or assign it
to a named constant and export it) so the component shows up with a proper name
in tooling and error traces — update the declaration where export default
function() appears (the component in src/features/Search/index.tsx) to use a
named function or named const and export that name.

---

10-10: Consider naming the default-exported component for easier debugging.

export default function() produces an anonymous component, making it harder to identify in SolidJS DevTools and stack traces.

♻️ Proposed refactor

-export default function() {
+export default function Search() {

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/features/Search/index.tsx` at line 10, The default-exported anonymous
component should be given a name to aid debugging and DevTools; replace the
anonymous default export in src/features/Search/index.tsx with a named component
(e.g., "Search") — either declare a named function component Search and export
it as the default or assign the component to a const Search and export default
Search — ensuring the component identifier (Search) is used so stack traces and
SolidJS DevTools show the component name.

package.json (1)

29-29: Operational risk: youtubei.js uses YouTube's undocumented private (InnerTube) API.

youtubei.js is not affiliated with, endorsed, or sponsored by YouTube. Relying on an undocumented internal API carries inherent stability and ToS risks: YouTube can change the API contract or block clients without notice. Ensure this trade-off is understood and that there's a mitigation plan (e.g., version-locking, fallback routes) for when the API breaks.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@package.json` at line 29, The dependency "youtubei.js" in package.json relies
on YouTube's undocumented InnerTube API and poses operational/ToS risk; update
package.json to mitigate by pinning to a specific tested version of
"youtubei.js" (replace the caret range with an exact version), add a clear
comment/README entry and a runtime feature flag or config toggle (e.g.,
YOUTUBE_PRIVATE_API_ENABLED) to allow quick rollback/disable, implement a
fallback path or graceful degradation in the code that uses youtubei.js
(identify the module/function that calls it and add error handling and an
alternate flow), and add monitoring/alerting around failures so breaking changes
are detected early.

src/lib/modules/audioErrorHandler.ts (1)

31-33: Playback is resumed via external event handler, but the design is implicit and fragile.

When audio.src changes on line 32, it triggers an implicit load algorithm that fires the onloadstart event. The player store has an onloadstart handler (src/lib/stores/player.ts:146–154) that calls audio.play() if isPlayable is true.

However, this design is fragile because:

• The audioErrorHandler function does not explicitly restore playback state; it relies entirely on the external onloadstart listener.
• Playback resumption depends on the isPlayable flag, which is module-level state that becomes true only when history exists or specific URL params are present.
• Future refactors that remove or modify the onloadstart handler could silently break proxy recovery.

Consider having audioErrorHandler explicitly resume playback based on the original playback state, rather than relying on implicit external event handling.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/lib/modules/audioErrorHandler.ts` around lines 31 - 33, The
audioErrorHandler currently swaps audio.src and relies on the player store's
onloadstart handler to resume playback; instead, capture the original playback
intent inside audioErrorHandler (e.g., wasPlaying = !audio.paused or a saved
playback flag) before replacing src, perform the proxy swap (audio.src = ...),
then explicitly attempt to resume if wasPlaying by calling audio.play()
(handling the returned promise and awaiting canplay/loadedmetadata if needed) so
recovery does not depend on the external onloadstart handler or the module-level
isPlayable flag; update audioErrorHandler to manage this resume logic and remove
the implicit dependency on the player store’s onloadstart.

netlify/edge-functions/syncHash.ts (1)

3-57: Coding-guideline violation: async/await and try/catch used throughout.

The entire handler uses async/await and try/catch, which conflicts with the project's guideline of preferring Promise-chain .then().catch() syntax.

As per coding guidelines, "Prefer Promise-based .then().catch() syntax over async/await or try...catch blocks."

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@netlify/edge-functions/syncHash.ts` around lines 3 - 57, The handler
currently uses async/await and try/catch (default exported async function, await
req.json(), await fetch(...), await crypto.subtle.digest(...)), violating the
project's guideline; refactor the default export to use Promise chains instead:
replace await req.json() with req.json().then(...), perform the email
verification using fetch(validatorUrl).then(res => res.json()).then(data => {
check data.status === 'VALID' }).catch(...) to handle fetch errors, and compute
the SHA-256 via crypto.subtle.digest(...).then(hashBuffer => { convert to hex
string and return new Response(...) }). Ensure all error cases return the
appropriate Response inside .then/.catch handlers rather than using
throw/try/catch, and keep existing identifiers (validatorUrl, normalizedEmail,
combinedString, hashedPassword) for clarity.

src/lib/utils/library.ts (2)

107-114: metaUpdater('albums') called even when album was already present.

When the album already exists (line 109 condition fails, push is skipped), metaUpdater('albums') on line 113 still fires, updating the timestamp and scheduling a sync for no actual change. Move it inside the conditional:

Proposed fix

 export function saveAlbumToLibrary(albumId: string, albumData: Album) {
   const albums = getLibraryAlbums();
   if (!albums.find(a => a.id === albumId)) {
     albums.push(albumData);
     saveLibraryAlbums(albums);
+    metaUpdater('albums');
   }
-  metaUpdater('albums');
 }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/lib/utils/library.ts` around lines 107 - 114, The function
saveAlbumToLibrary currently calls metaUpdater('albums') unconditionally,
causing metadata updates even when no album was added; modify saveAlbumToLibrary
so that metaUpdater('albums') is invoked only inside the block where a new album
is pushed and saveLibraryAlbums(albums) is called (i.e., after the if
(!albums.find(a => a.id === albumId)) { ... } branch), leaving getLibraryAlbums
and saveLibraryAlbums usage unchanged.

---

428-479: cleanseLibraryData only validates non-empty IDs, not track-map membership.

Line 474 filters collection entries with collection.filter(id => id) (removes falsy values), but doesn't check whether each ID actually exists in cleanedTracks. Collections can retain IDs pointing to tracks that were just removed in step 3 (lines 446-466). Consider also filtering against the cleaned tracks map:

Proposed fix

-    const validCollection = collection.filter(id => id);
+    const validCollection = collection.filter(id => id && cleanedTracks[id]);

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/lib/utils/library.ts` around lines 428 - 479, cleanseLibraryData
currently only removes falsy IDs from each collection but doesn't ensure IDs
actually exist in the cleaned track map; update the collection-cleaning loop to
also validate membership against the final set of valid track IDs. After step 3,
build a Set of valid IDs (use Object.keys(cleanedTracks) if tracksCleaned is
true, otherwise use referencedTrackIds) and then change the filter in the for
loop that calls saveCollection(key, validCollection) to filter by id &&
validIds.has(id); keep the existing logging and saveCollection call.

src/lib/modules/cloudSync.ts (3)

13-23: Avoid any in type definitions.

LibrarySnapshot and DeltaPayload use any in multiple places. Per coding guidelines, prefer fully-typed definitions. Consider typing them more specifically — e.g., unknown at minimum, or a union of the known stored shapes.

 interface LibrarySnapshot {
-  [key: string]: any;
+  [key: string]: unknown;
 }

 interface DeltaPayload {
   meta: Partial<Meta>;
   addedOrUpdatedTracks: Collection;
   deletedTrackIds: string[];
-  updatedCollections: { [collectionName: string]: any };
+  updatedCollections: { [collectionName: string]: unknown };
   deletedCollectionNames: string[];
 }

As per coding guidelines, "Use fully-typed, generic, and reusable code over quick fixes; avoid the use of any type".

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/lib/modules/cloudSync.ts` around lines 13 - 23, LibrarySnapshot and
DeltaPayload currently use `any`; replace those with concrete types (or at
minimum `unknown`) and introduce specific interfaces or generics for the known
stored shapes to satisfy the typing rules: update LibrarySnapshot to map string
keys to a union or generic type representing allowed namespaces (e.g., Meta,
TracksCollection, FavoritesCollection) instead of `any`, and change DeltaPayload
fields (addedOrUpdatedTracks, updatedCollections) to use the existing Collection
type more precisely or a new Collection<T> generic and type updatedCollections
as a record of specific collection value types rather than `{ [collectionName:
string]: any }`; likewise replace `deletedCollectionNames: string[]` usage if
needed with a more explicit enum/union of allowed collection names. Reference
the types LibrarySnapshot, DeltaPayload, Meta, and Collection to locate where to
change and create new small interfaces/generics for the stored shapes, or use
`unknown` as a minimum safe replacement for `any`.

---

30-73: Prefer .then().catch() over async/await + try/catch.

Both pullFullLibrary and pushFullLibrary use async/await. The coding guidelines prefer Promise-based .then().catch() syntax. This applies to the changed segments in these functions (lines 53-62 in pushFullLibrary). Consider refactoring the changed functions accordingly.

As per coding guidelines, "Prefer Promise-based .then().catch() syntax over async/await or try...catch blocks".

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/lib/modules/cloudSync.ts` around lines 30 - 73, Refactor pullFullLibrary
and pushFullLibrary to use Promise .then().catch() chains instead of async/await
and try/catch: for pullFullLibrary replace the await fetch and await
response.json() with fetch(...).then(response => { if (!response.ok) throw new
Error(...); return response.json(); }).then(snapshot => { clear localStorage
keys that startWith('library_') and write snapshot entries }).catch(err => {
throw err; }); for pushFullLibrary keep building the snapshot but replace the
JSON.parse try/catch per key with Promise.resolve(val).then(v =>
JSON.parse(v)).then(parsed => { snapshot[key.slice(8)] = parsed }).catch(e =>
console.warn(...)); then perform fetch(`/library/${userId}`, { method: 'PUT',
... , body: JSON.stringify(snapshot) }).then(response => { if (!response.ok)
throw new Error(...); }).catch(err => { throw err; }). Ensure you reference the
existing function names pullFullLibrary and pushFullLibrary and the 'library_'
key prefix when updating the logic.

---

281-297: Remove redundant scheduleSync() call after metaUpdater('tracks').

metaUpdater('tracks') calls syncLibrary('schedule'), which itself calls scheduleSync(). The explicit scheduleSync() calls on lines 287 and 296 are therefore redundant. Both calls are idempotent (the timeout is cleared and reset), so this isn't a bug, but removing the direct calls would clarify the code.

Proposed fix

 export const addDirtyTrack = (id: string) => {
   const dirtyTracks = getDirtyTracks();
   if (!dirtyTracks.added.includes(id)) dirtyTracks.added.push(id);
   dirtyTracks.deleted = dirtyTracks.deleted.filter((deletedId) => deletedId !== id);
   saveDirtyTracks(dirtyTracks);
   metaUpdater('tracks');
-  scheduleSync();
 };

 export const removeDirtyTrack = (id: string) => {
   const dirtyTracks = getDirtyTracks();
   if (!dirtyTracks.deleted.includes(id)) dirtyTracks.deleted.push(id);
   dirtyTracks.added = dirtyTracks.added.filter((addedId) => addedId !== id);
   saveDirtyTracks(dirtyTracks);
   metaUpdater('tracks');
-  scheduleSync();
 };

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/lib/modules/cloudSync.ts` around lines 281 - 297, The addDirtyTrack and
removeDirtyTrack functions call scheduleSync() immediately after
metaUpdater('tracks'), but metaUpdater('tracks') invokes syncLibrary('schedule')
which already calls scheduleSync(), so remove the redundant explicit
scheduleSync() calls in both addDirtyTrack and removeDirtyTrack; keep the rest
of the logic (getDirtyTracks, saveDirtyTracks, metaUpdater('tracks')) unchanged
so metaUpdater triggers scheduling centrally.

src/components/ActionsMenu/index.tsx (1)

100-100: async/await/try/finally pattern in download, view-author, and view-album handlers violates coding guidelines; also async on the radio handler is a no-op.

• Line 100: async () => with no await in the body — the radio handler uses a Promise chain correctly, but the async modifier is redundant and misleading.
• Lines 128–147, 149–175, 179–193: All three handlers use async/await with try/finally instead of the mandated Promise chain.

Refactor to the Promise-based pattern (example for view-author):

♻️ Proposed refactor (view-author, lines 149–175)

-       <li tabindex="5" onclick={async () => {
+       <li tabindex="5" onclick={() => {
          if (isViewingAuthor()) return;
          const { author, authorId } = store.actionsMenu as TrackItem;

          if (author)
            setListStore('name',
              author.endsWith('- Topic') ?
                ('Artist - ' + author.replace('- Topic', ''))
                : '');

          if (authorId) {
            setIsViewingAuthor(true);
-           try {
-             await getList(authorId, isMusic ? 'artist' : 'channel');
-           } finally {
-             setIsViewingAuthor(false);
-             closeDialog();
-           }
+           getList(authorId, isMusic ? 'artist' : 'channel')
+             .finally(() => {
+               setIsViewingAuthor(false);
+               closeDialog();
+             });
          }
        }}>

Apply the same pattern to the download (getDownloadLink(id).finally(...)) and view-album handlers.

As per coding guidelines, **/*.{ts,tsx} requires: "Prefer Promise-based .then().catch() syntax over async/await or try...catch blocks."

Also applies to: 128-147, 149-175, 179-193

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/components/ActionsMenu/index.tsx` at line 100, The inline onclick
handlers in ActionsMenu (the radio handler using an unnecessary async modifier,
and the download, view-author, and view-album handlers using async/await with
try/finally) violate the project guideline requiring Promise chains; remove the
redundant async from the radio handler and refactor the three async functions to
return Promise-based chains (use .then()/ .catch()/ .finally() as appropriate) —
e.g., replace await/try/finally flows around getDownloadLink(id), the viewAuthor
logic, and the viewAlbum logic with
getDownloadLink(id).then(...).catch(...).finally(...) (or equivalent
.then().finally()) so no async/await or try/finally remains in those onclick
handlers.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@netlify/edge-functions/syncHash.ts`:
- Around line 45-46: The change to compute combinedString as
`${normalizedEmail}|${password}` breaks existing keys; implement a versioned
lookup/migration in the syncHash flow so existing blobs aren't orphaned: compute
both the legacy hash (use raw email+password as previously computed) and the new
hash (using normalizedEmail and the '|' separator), try fetching the blob under
the legacy key first, and if found copy/rename it to the new key (or create a
"v2:<new_hash>" pointer) then continue using the new key; modify the code paths
around normalizedEmail and combinedString to perform the legacy lookup (and
migrate on first successful hit) before creating new blobs so old user data
remains accessible.

In `@package.json`:
- Line 29: The package "youtubei.js" is a runtime dependency (used by
src/backend/getArtist.ts and src/backend/getSearch.ts) and must be moved from
devDependencies to dependencies in package.json: update package.json to remove
"youtubei.js" from devDependencies and add it under dependencies with the same
version specifier, then run a fresh install to update lockfile; also add a short
note in the repo (e.g., README or docs) flagging that youtubei.js uses YouTube’s
InnerTube API so maintainers can review any terms-of-service/legal risk.

In `@src/components/ActionsMenu/index.tsx`:
- Line 41: The dialog backdrop click handler should also check the radio loading
state so it doesn't close while a radio fetch is in-progress: add
queueStore.isLoading to the existing guard alongside isDownloading(),
isViewingAuthor(), and isViewingAlbum() before calling closeDialog(). For the
radio onclick (the handler that calls getRadio), add an early-return guard that
prevents starting a new fetch when queueStore.isLoading (or a dedicated
inProgress flag) is true, and set/clear that flag around the getRadio call (or
otherwise serialize the promise) so rapid clicks cannot spawn concurrent
getRadio requests that overwrite the queue.
- Around line 30-34: The effect in createEffect is destructuring
store.actionsMenu as TrackItem which can be undefined when closeDialog() calls
setStore('actionsMenu', undefined), causing a TypeError; update the effect in
the ActionsMenu component to first check that store.actionsMenu is defined (or
use optional chaining) before destructuring/reading id and only call
setIsListenLater(getCollection('listenLater').includes(id)) when id exists —
e.g., guard against undefined store.actionsMenu or early return so
setIsListenLater is not called with an undefined id.

In `@src/lib/utils/library.ts`:
- Around line 5-12: The dynamic import in syncLibrary currently omits error
handling and can cause unhandled promise rejections; update the
import('@lib/modules/cloudSync') call in syncLibrary (which gates on
config.dbsync) to append a .catch(...) handler that logs the error and
optionally suppresses it (e.g., console.error or the project logger) so failures
loading the cloudSync module do not reject unhandled; keep the existing
.then(...) behavior that invokes addDirtyTrack, removeDirtyTrack, and
scheduleSync.

---

Outside diff comments:
In `@netlify/edge-functions/syncHash.ts`:
- Line 21: The validator URL is constructed without encoding user input which
allows characters like "+", "&", "#" or spaces in the email to break the query
string; update the construction of validatorUrl (in syncHash.ts where
validatorUrl is defined) to URL-encode the email using encodeURIComponent(email)
before interpolation so the query parameter is safe and cannot inject extra
params or corrupt the URL.
- Line 11: The call to req.json() in syncHash is unprotected and can throw on
invalid JSON; wrap the await req.json() in a try/catch inside the syncHash
handler (or the function that contains the line "const { email, password } =
await req.json();"), catch JSON parse errors and return a 400 Bad Request
response with a clear error message, and only destructure email/password after
validating the parsed body; ensure other errors still propagate or return 500 as
appropriate.
- Around line 48-52: The code in syncHash.ts currently uses
crypto.subtle.digest('SHA-256') (the const hashBuffer calculation) which is
inappropriate for password hashing; replace that block with a PBKDF2 derivation:
generate or accept a per-user salt, define a constant iterations count, import
the password as a raw key via crypto.subtle.importKey, then call
crypto.subtle.deriveBits with { name: 'PBKDF2', salt, iterations, hash:
'SHA-256' } to derive the hash bytes and convert to hex; update storage to
persist the salt and iterations (and handle the separator/migration for existing
SHA-256 hashes). Ensure you update any functions referencing hashedPassword/sync
logic to expect PBKDF2 output and include salt+iterations metadata for
verification.

---

Duplicate comments:
In `@src/features/Search/index.tsx`:
- Line 8: The lazy-loaded About component (const About = lazy(() =>
import('./About'))) is being rendered directly (used as the fallback of <Show>)
without a Suspense boundary; import Suspense from 'solid-js' and wrap the <About
/> render sites (particularly where it's used as the fallback in the <Show>
around the Search component and similarly at lines ~56-58) in <Suspense
fallback={...}> so the lazy import can suspend safely; ensure the fallback
passed to Suspense is an appropriate loader and remove the bare <About /> usage.
- Line 8: The lazy-loaded About component is rendered without a local Suspense
boundary and will cause the nearest (possibly root) Suspense to suspend; import
Suspense from 'solid-js' and wrap the lazy About element with a local <Suspense
fallback={...}> (use the same or a minimal spinner/fallback used by the parent
Show) wherever About is returned as the Show's fallback (the code that renders
<Show fallback={...}><About /></Show>); ensure the import and the Suspense
wrapper replace the bare About usage so the lazy load only suspends that local
boundary.

In `@src/features/Search/Search.css`:
- Around line 4-6: The .right-group rule uses var(--size-000) which is negative
(Open Props sets --size-000 = -0.5rem) causing the element to be pulled outside
the header; update the .right-group CSS to use a non-negative spacing token
(e.g., replace var(--size-000) with a zero or positive token such as
var(--size-0) or remove the margin-right entirely) so the element stays inside
the container and layout is not broken.

In `@src/lib/modules/audioErrorHandler.ts`:
- Line 10: The code constructs const url = new URL(audio.src) before checking
the early-return guard for the "&fallback" case, which can throw a TypeError
when audio.src is empty; move the URL construction so it occurs after the
"&fallback" early-return (after the guard around line 18) and only when
audio.src is non-empty, i.e., remove or relocate the const url = new
URL(audio.src) statement and reintroduce it below the fallback guard where
further processing requires a URL.

In `@src/lib/modules/libraryMigratorV5.ts`:
- Around line 76-81: The migration unconditionally sets meta.version = 5, writes
localStorage and triggers setStore('snackbar', ...)/location.reload() even if
one or more per-section migrations (channels, playlists, albums) failed; capture
overall success by tracking each migration's outcome (e.g., booleans or a single
success flag updated inside the existing try/catch blocks for the
channels/playlists/albums migration functions), and only perform meta.version =
5, localStorage.setItem('library_meta', ...), setStore('snackbar', ...) and
location.reload() when that overall success flag is true; also move or remove
the console.log so it runs before calling location.reload() (or keep it and
ensure it executes before reload), and make sure failed migrations leave
meta.version at 4 so retries are possible on next load.
- Around line 16-18: OldAlbum declares tracks?: string[] but the album migration
loop never reads or maps it; either remove tracks from the OldAlbum interface to
avoid confusing dead fields, or preserve the data by updating the album
migration loop to copy OldAlbum.tracks into the target album shape (e.g., map to
the new album's tracks/trackIds field) during migration. Locate the
OldAlbum/OldPlaylist definitions and the album migration loop in
libraryMigratorV5.ts and either delete the tracks property from the interface or
add a mapping line in the loop to transfer oldAlbum.tracks into the new album
object so the data is not dropped.

In `@src/lib/utils/library.ts`:
- Around line 336-341: The bug is that ids.map(id => tracks[id]) can produce
undefined for orphaned IDs and crash sortCollection; update the block that
builds items before calling sortCollection (the code that computes
items/sortedItems/sortedIds using ids, tracks, sortCollection) to filter out
falsy/undefined entries (e.g., use .map(...).filter(Boolean) or equivalent) so
only valid track objects reach sortCollection; ensure sortedIds is derived from
the filtered sortedItems and keep the existing reserved/ordering checks
(isReserved/listStore.reservedCollections, config.sortBy, config.sortOrder)
intact.
- Around line 46-49: getLibraryAlbums() now returns an Album[] but the code
checks presence with Object.keys(albums).length; change that to albums.length to
correctly and clearly test the array length. Locate the block where albums is
assigned from getLibraryAlbums() in src/lib/utils/library.ts (variable albums)
and replace the Object.keys(albums).length check with albums.length while
keeping the existing assignment to newMeta.albums = timestamp when the array is
non-empty.
- Around line 350-368: The code in the usePagination branch and the else branch
unconditionally sets type: 'video' when building collection items, which
overwrites any existing track type (e.g., 'song'); update the mappings inside
setListStore (both the initial slice, the observerCallback nextBatch mapping,
and the non-paginated mapping) to preserve a track's existing type
(tracks[id].type) or fall back to 'video' only if missing, and ensure
TrackItem/Collection types include the type field so it round-trips correctly;
locate the mappings in the usePagination block (loadedCount, observerCallback,
setObserver) and the final else branch where sortedIds.map(...) is used and
change them to use the stored type instead of hardcoding 'video'.

In `@src/types.d.ts`:
- Line 71: The Collection type currently maps IDs to TrackItem, which drops
YTItem-specific fields (img, albumId, subtext, type) and causes
getLocalCollection in library.ts to force type: 'video', corrupting song/video
info; change the Collection alias in src/types.d.ts to map IDs to YTItem (or at
minimum a union that includes the type field) so saved JSON preserves those
fields, and then update usages such as addToCollection and getLocalCollection to
accept/return Collection<{...}> with the wider type (ensure getLocalCollection
no longer hardcodes type: 'video' and instead reads stored type) so the type
system reflects stored data.

---

Nitpick comments:
In `@netlify/edge-functions/syncHash.ts`:
- Around line 3-57: The handler currently uses async/await and try/catch
(default exported async function, await req.json(), await fetch(...), await
crypto.subtle.digest(...)), violating the project's guideline; refactor the
default export to use Promise chains instead: replace await req.json() with
req.json().then(...), perform the email verification using
fetch(validatorUrl).then(res => res.json()).then(data => { check data.status ===
'VALID' }).catch(...) to handle fetch errors, and compute the SHA-256 via
crypto.subtle.digest(...).then(hashBuffer => { convert to hex string and return
new Response(...) }). Ensure all error cases return the appropriate Response
inside .then/.catch handlers rather than using throw/try/catch, and keep
existing identifiers (validatorUrl, normalizedEmail, combinedString,
hashedPassword) for clarity.

In `@package.json`:
- Line 29: The dependency "youtubei.js" in package.json relies on YouTube's
undocumented InnerTube API and poses operational/ToS risk; update package.json
to mitigate by pinning to a specific tested version of "youtubei.js" (replace
the caret range with an exact version), add a clear comment/README entry and a
runtime feature flag or config toggle (e.g., YOUTUBE_PRIVATE_API_ENABLED) to
allow quick rollback/disable, implement a fallback path or graceful degradation
in the code that uses youtubei.js (identify the module/function that calls it
and add error handling and an alternate flow), and add monitoring/alerting
around failures so breaking changes are detected early.

In `@src/components/ActionsMenu/index.tsx`:
- Line 100: The inline onclick handlers in ActionsMenu (the radio handler using
an unnecessary async modifier, and the download, view-author, and view-album
handlers using async/await with try/finally) violate the project guideline
requiring Promise chains; remove the redundant async from the radio handler and
refactor the three async functions to return Promise-based chains (use .then()/
.catch()/ .finally() as appropriate) — e.g., replace await/try/finally flows
around getDownloadLink(id), the viewAuthor logic, and the viewAlbum logic with
getDownloadLink(id).then(...).catch(...).finally(...) (or equivalent
.then().finally()) so no async/await or try/finally remains in those onclick
handlers.

In `@src/features/Search/index.tsx`:
- Line 10: The default-exported component is anonymous (export default
function()) which hinders identification in SolidJS DevTools and stack traces;
rename the component (e.g., give the function a meaningful identifier like
Search) and export that named function (or assign it to a named constant and
export it) so the component shows up with a proper name in tooling and error
traces — update the declaration where export default function() appears (the
component in src/features/Search/index.tsx) to use a named function or named
const and export that name.
- Line 10: The default-exported anonymous component should be given a name to
aid debugging and DevTools; replace the anonymous default export in
src/features/Search/index.tsx with a named component (e.g., "Search") — either
declare a named function component Search and export it as the default or assign
the component to a const Search and export default Search — ensuring the
component identifier (Search) is used so stack traces and SolidJS DevTools show
the component name.

In `@src/lib/modules/audioErrorHandler.ts`:
- Around line 31-33: The audioErrorHandler currently swaps audio.src and relies
on the player store's onloadstart handler to resume playback; instead, capture
the original playback intent inside audioErrorHandler (e.g., wasPlaying =
!audio.paused or a saved playback flag) before replacing src, perform the proxy
swap (audio.src = ...), then explicitly attempt to resume if wasPlaying by
calling audio.play() (handling the returned promise and awaiting
canplay/loadedmetadata if needed) so recovery does not depend on the external
onloadstart handler or the module-level isPlayable flag; update
audioErrorHandler to manage this resume logic and remove the implicit dependency
on the player store’s onloadstart.

In `@src/lib/modules/cloudSync.ts`:
- Around line 13-23: LibrarySnapshot and DeltaPayload currently use `any`;
replace those with concrete types (or at minimum `unknown`) and introduce
specific interfaces or generics for the known stored shapes to satisfy the
typing rules: update LibrarySnapshot to map string keys to a union or generic
type representing allowed namespaces (e.g., Meta, TracksCollection,
FavoritesCollection) instead of `any`, and change DeltaPayload fields
(addedOrUpdatedTracks, updatedCollections) to use the existing Collection type
more precisely or a new Collection<T> generic and type updatedCollections as a
record of specific collection value types rather than `{ [collectionName:
string]: any }`; likewise replace `deletedCollectionNames: string[]` usage if
needed with a more explicit enum/union of allowed collection names. Reference
the types LibrarySnapshot, DeltaPayload, Meta, and Collection to locate where to
change and create new small interfaces/generics for the stored shapes, or use
`unknown` as a minimum safe replacement for `any`.
- Around line 30-73: Refactor pullFullLibrary and pushFullLibrary to use Promise
.then().catch() chains instead of async/await and try/catch: for pullFullLibrary
replace the await fetch and await response.json() with fetch(...).then(response
=> { if (!response.ok) throw new Error(...); return response.json();
}).then(snapshot => { clear localStorage keys that startWith('library_') and
write snapshot entries }).catch(err => { throw err; }); for pushFullLibrary keep
building the snapshot but replace the JSON.parse try/catch per key with
Promise.resolve(val).then(v => JSON.parse(v)).then(parsed => {
snapshot[key.slice(8)] = parsed }).catch(e => console.warn(...)); then perform
fetch(`/library/${userId}`, { method: 'PUT', ... , body:
JSON.stringify(snapshot) }).then(response => { if (!response.ok) throw new
Error(...); }).catch(err => { throw err; }). Ensure you reference the existing
function names pullFullLibrary and pushFullLibrary and the 'library_' key prefix
when updating the logic.
- Around line 281-297: The addDirtyTrack and removeDirtyTrack functions call
scheduleSync() immediately after metaUpdater('tracks'), but
metaUpdater('tracks') invokes syncLibrary('schedule') which already calls
scheduleSync(), so remove the redundant explicit scheduleSync() calls in both
addDirtyTrack and removeDirtyTrack; keep the rest of the logic (getDirtyTracks,
saveDirtyTracks, metaUpdater('tracks')) unchanged so metaUpdater triggers
scheduling centrally.

In `@src/lib/utils/library.ts`:
- Around line 107-114: The function saveAlbumToLibrary currently calls
metaUpdater('albums') unconditionally, causing metadata updates even when no
album was added; modify saveAlbumToLibrary so that metaUpdater('albums') is
invoked only inside the block where a new album is pushed and
saveLibraryAlbums(albums) is called (i.e., after the if (!albums.find(a => a.id
=== albumId)) { ... } branch), leaving getLibraryAlbums and saveLibraryAlbums
usage unchanged.
- Around line 428-479: cleanseLibraryData currently only removes falsy IDs from
each collection but doesn't ensure IDs actually exist in the cleaned track map;
update the collection-cleaning loop to also validate membership against the
final set of valid track IDs. After step 3, build a Set of valid IDs (use
Object.keys(cleanedTracks) if tracksCleaned is true, otherwise use
referencedTrackIds) and then change the filter in the for loop that calls
saveCollection(key, validCollection) to filter by id && validIds.has(id); keep
the existing logging and saveCollection call.

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

# First, read the syncHash.ts file to understand the full context
cat -n netlify/edge-functions/syncHash.ts

Repository: n-ce/ytify

Length of output: 2289

---

🏁 Script executed:

# Search for references to the /hash endpoint and hash storage/comparison patterns
rg -n --type=ts --type=js -C3 '"/hash"'

Repository: n-ce/ytify

Length of output: 673

---

🏁 Script executed:

# Search for hashedPassword, syncHash, and similar patterns
rg -n --type=ts --type=js -C3 'hashedPassword|syncHash|hash.*password'

Repository: n-ce/ytify

Length of output: 1287

---

🏁 Script executed:

# Look for any migration or versioning logic
rg -n --type=ts --type=js -C3 'migration|version|v1:|v2:|rehash'

Repository: n-ce/ytify

Length of output: 10331

---

🏁 Script executed:

# Find where the hash result is used after fetch
rg -n --type=ts --type=tsx -A5 'fetch.*\/hash'

Repository: n-ce/ytify

Length of output: 140

---

🏁 Script executed:

# Search for hash validation or comparison logic
rg -n --type=ts --type=tsx --type=js 'hashedPassword|hash.*compar|hash.*validat'

Repository: n-ce/ytify

Length of output: 81

---

🏁 Script executed:

# Check the sync.ts endpoint for hash validation
head -80 netlify/edge-functions/sync.ts

Repository: n-ce/ytify

Length of output: 2757

---

🏁 Script executed:

# Search for authentication or credential validation in backend
rg -n --type=ts 'password|credential|auth' netlify/edge-functions/

Repository: n-ce/ytify

Length of output: 881

---

🏁 Script executed:

# Find where hash is passed to sync endpoint
rg -n -A10 'fetch.*hash' src/

Repository: n-ce/ytify

Length of output: 687

---

🏁 Script executed:

# Look at the full Login.tsx to see what happens with the hash result
sed -n '15,50p' src/components/Login.tsx

Repository: n-ce/ytify

Length of output: 1072

---

🏁 Script executed:

# Check if hash is stored in localStorage or state
rg -n 'hash.*localStorage|localStorage.*hash|setHash|hash.*store' src/

Repository: n-ce/ytify

Length of output: 36

---

🏁 Script executed:

# Search in all TypeScript/JavaScript files for how hash is used after fetch
rg -n -A5 'const.*hash.*response|response.*hash|hashedPassword.*=' src/ netlify/

Repository: n-ce/ytify

Length of output: 453

---

🏁 Script executed:

# Find the cloudSync.runSync function to see how the hash is used
rg -n 'runSync|export.*runSync' src/lib/modules/

Repository: n-ce/ytify

Length of output: 243

---

🏁 Script executed:

# Check the full cloudSync.ts file to understand the flow
head -100 src/lib/modules/cloudSync.ts

Repository: n-ce/ytify

Length of output: 3183

---

🏁 Script executed:

# See if there's any logic that tries both old and new hash formats
rg -n 'userIdHash|context.params.hash' netlify/

Repository: n-ce/ytify

Length of output: 1577

---

🏁 Script executed:

# Check if there's any logic to handle missing blobs or fallback to old hash format
grep -n 'userIdHash\|hash' netlify/edge-functions/sync.ts | head -30

Repository: n-ce/ytify

Length of output: 849

---

🏁 Script executed:

# Look for any migration, rehash, or compatibility logic in the codebase
rg -n 'oldHash|rehash|migrate.*hash|fallback.*hash' netlify/ src/

Repository: n-ce/ytify

Length of output: 36

---

🏁 Script executed:

# Check if there's any code that attempts to load data with an alternative hash
rg -n 'getWithMetadata|libraryStore.get' netlify/edge-functions/

Repository: n-ce/ytify

Length of output: 544

---

🏁 Script executed:

# Verify the endpoint path routing to see how hash is used as a parameter
grep -n 'path:' netlify/edge-functions/sync.ts netlify/edge-functions/library.ts

Repository: n-ce/ytify

Length of output: 178

---

🏁 Script executed:

# Check if there's any error handling that might fall back to old format
sed -n '50,70p' netlify/edge-functions/sync.ts

Repository: n-ce/ytify

Length of output: 740

---

🏁 Script executed:

# Look for any documentation or comments about hash scheme
rg -n 'userIdHash|hash.*scheme|hash.*version' --type=ts -B2 -A2 netlify/

Repository: n-ce/ytify

Length of output: 4928

---

Breaking change: all existing stored hashes are immediately invalidated.

The hash input changed from email+password to normalizedEmail|password — a different byte sequence that produces a completely different SHA-256 digest. This hash is used as the storage key in Netlify Blobs (/sync/:hash and /library/:hash). Every user whose library was synced under the old hash format will have their data orphaned; a new login with the same credentials will compute a different hash and create a new, empty library entry with no access to the previous data.

Before deploying to production, implement one of:

• A migration that re-hashes existing blob keys from old to new format, or
• A versioned hash scheme (v2:<new_hash>) that attempts lookup under old format first, then re-hashes on first successful login.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@netlify/edge-functions/syncHash.ts` around lines 45 - 46, The change to
compute combinedString as `${normalizedEmail}|${password}` breaks existing keys;
implement a versioned lookup/migration in the syncHash flow so existing blobs
aren't orphaned: compute both the legacy hash (use raw email+password as
previously computed) and the new hash (using normalizedEmail and the '|'
separator), try fetching the blob under the legacy key first, and if found
copy/rename it to the new key (or create a "v2:<new_hash>" pointer) then
continue using the new key; modify the code paths around normalizedEmail and
combinedString to perform the legacy lookup (and migrate on first successful
hit) before creating new blobs so old user data remains accessible.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check vite config
echo "=== Vite config files ==="
fd -e ts -e js -e mjs 'vite\.config' -x cat {}

echo ""
echo "=== Netlify/Vercel config ==="
fd -g 'netlify.toml' -x cat {}
fd -g 'vercel.json' -x cat {}

echo ""
echo "=== Check for youtubei.js imports in API routes ==="
rg -t ts "from ['\"]youtubei" api/

echo ""
echo "=== List API route files to understand structure ==="
fd -t f 'api/.*\.ts' | head -20

Repository: n-ce/ytify

Length of output: 5408

---

🏁 Script executed:

#!/bin/bash
# Search for any usage of youtubei across the entire codebase
echo "=== All references to 'youtubei' ==="
rg -i "youtubei" --type ts --type js

echo ""
echo "=== API directory structure ==="
ls -la api/ 2>/dev/null || echo "api/ directory does not exist"

echo ""
echo "=== Check src/backend/ structure ==="
fd -t f . src/backend/ 2>/dev/null | head -20

echo ""
echo "=== Check entire src structure ==="
find src -type d | head -30

Repository: n-ce/ytify

Length of output: 2193

---

🏁 Script executed:

#!/bin/bash
# Check how api/ files import from src/backend/
echo "=== api/artist.ts content ==="
cat api/artist.ts

echo ""
echo "=== api/_helper.ts content ==="
cat api/_helper.ts

echo ""
echo "=== Check package.json build/deploy scripts ==="
jq '.scripts' package.json

echo ""
echo "=== Check Vercel serverless function configuration ==="
find . -name 'vercel.json' -o -name 'netlify.toml' | xargs cat 2>/dev/null

Repository: n-ce/ytify

Length of output: 1800

---

youtubei.js should be in dependencies, not devDependencies, despite Vercel bundling both.

Vercel automatically bundles all packages—both dependencies and devDependencies—into serverless function bundles at deploy time, so the immediate runtime concern in the original review is not an issue for this specific platform. However, youtubei.js is a runtime dependency (used in src/backend/getArtist.ts, getSearch.ts, etc., which are executed on every request), and best practice is to place runtime dependencies in dependencies. This improves portability: if the code is ever run on a different platform, deployed via container, or installed with npm install --production, the package will be available. It also clarifies the dependency graph for other developers.

Additionally, consider whether youtubei.js wrapping YouTube's private InnerTube API creates any terms-of-service or legal risk for a production service; this may warrant documentation or mitigation strategy.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@package.json` at line 29, The package "youtubei.js" is a runtime dependency
(used by src/backend/getArtist.ts and src/backend/getSearch.ts) and must be
moved from devDependencies to dependencies in package.json: update package.json
to remove "youtubei.js" from devDependencies and add it under dependencies with
the same version specifier, then run a fresh install to update lockfile; also
add a short note in the repo (e.g., README or docs) flagging that youtubei.js
uses YouTube’s InnerTube API so maintainers can review any
terms-of-service/legal risk.

[coderabbitai]

⚠️ Potential issue | 🔴 Critical

createEffect will throw TypeError when store.actionsMenu is set to undefined.

closeDialog() calls setStore('actionsMenu', undefined). Because createEffect tracks store.actionsMenu, it re-executes when that value changes. The order of runs among multiple effects is not guaranteed, so this effect may fire before the parent <Show> tears down the component — destructuring { id } from undefined throws at runtime.

🐛 Proposed fix

  createEffect(() => {
-   const { id } = store.actionsMenu as TrackItem;
+   const id = store.actionsMenu?.id;
    if (id)
      setIsListenLater(getCollection('listenLater').includes(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.

Suggested change

	createEffect(() => {
	const { id } = store.actionsMenu as CollectionItem;
	const { id } = store.actionsMenu as TrackItem;
	if (id)
	setIsListenLater(getCollection('listenLater').includes(id));
	})
	createEffect(() => {
	const id = store.actionsMenu?.id;
	if (id)
	setIsListenLater(getCollection('listenLater').includes(id));
	})

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/components/ActionsMenu/index.tsx` around lines 30 - 34, The effect in
createEffect is destructuring store.actionsMenu as TrackItem which can be
undefined when closeDialog() calls setStore('actionsMenu', undefined), causing a
TypeError; update the effect in the ActionsMenu component to first check that
store.actionsMenu is defined (or use optional chaining) before
destructuring/reading id and only call
setIsListenLater(getCollection('listenLater').includes(id)) when id exists —
e.g., guard against undefined store.actionsMenu or early return so
setIsListenLater is not called with an undefined id.

[coderabbitai]

⚠️ Potential issue | 🟠 Major

Radio loading state not guarded in the dialog click-close handler, and the radio handler has no concurrent-click protection.

Two related defects:

1. Line 41's dialog backdrop guard omits queueStore.isLoading, so clicking outside the <ul> while radio is loading closes the dialog immediately — inconsistent with the download/view-author/view-album behaviors.
2. The radio onclick (line 100) has no early-return check, so rapid clicks start multiple concurrent getRadio fetches, each overwriting the queue independently.

🐛 Proposed fix

-     onclick={() => !isDownloading() && !isViewingAuthor() && !isViewingAlbum() && closeDialog()}
+     onclick={() => !isDownloading() && !isViewingAuthor() && !isViewingAlbum() && !queueStore.isLoading && closeDialog()}

  <li tabindex="3" onclick={async () => {
    const id = store.actionsMenu?.id;
    if (!id) return;
+   if (queueStore.isLoading) return;

    setQueueStore('isLoading', true);

Also applies to: 100-124

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/components/ActionsMenu/index.tsx` at line 41, The dialog backdrop click
handler should also check the radio loading state so it doesn't close while a
radio fetch is in-progress: add queueStore.isLoading to the existing guard
alongside isDownloading(), isViewingAuthor(), and isViewingAlbum() before
calling closeDialog(). For the radio onclick (the handler that calls getRadio),
add an early-return guard that prevents starting a new fetch when
queueStore.isLoading (or a dedicated inProgress flag) is true, and set/clear
that flag around the getRadio call (or otherwise serialize the promise) so rapid
clicks cannot spawn concurrent getRadio requests that overwrite the queue.

[coderabbitai]

🛠️ Refactor suggestion | 🟠 Major

Add .catch() to the dynamic import to prevent unhandled rejections.

If the cloudSync module fails to load (e.g., network error, bundling issue), the promise rejection is silently swallowed. Adding .catch() aligns with the coding guidelines and prevents unhandled promise rejections.

Proposed fix

 const syncLibrary = (action: 'add' | 'remove' | 'schedule', id?: string) => {
   if (!config.dbsync) return;
   import('@lib/modules/cloudSync').then(m => {
     if (action === 'add' && id) m.addDirtyTrack(id);
     else if (action === 'remove' && id) m.removeDirtyTrack(id);
     else if (action === 'schedule') m.scheduleSync();
-  });
+  }).catch(e => console.error('Failed to load cloudSync module:', e));
 };

As per coding guidelines, "Prefer Promise-based .then().catch() syntax over async/await or try...catch blocks".

📝 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.

Suggested change

	const syncLibrary = (action: 'add' | 'remove' | 'schedule', id?: string) => {
	if (!config.dbsync) return;
	import('@lib/modules/cloudSync').then(m => {
	if (action === 'add' && id) m.addDirtyTrack(id);
	else if (action === 'remove' && id) m.removeDirtyTrack(id);
	else if (action === 'schedule') m.scheduleSync();
	});
	};
	const syncLibrary = (action: 'add' | 'remove' | 'schedule', id?: string) => {
	if (!config.dbsync) return;
	import('@lib/modules/cloudSync').then(m => {
	if (action === 'add' && id) m.addDirtyTrack(id);
	else if (action === 'remove' && id) m.removeDirtyTrack(id);
	else if (action === 'schedule') m.scheduleSync();
	}).catch(e => console.error('Failed to load cloudSync module:', e));
	};

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/lib/utils/library.ts` around lines 5 - 12, The dynamic import in
syncLibrary currently omits error handling and can cause unhandled promise
rejections; update the import('@lib/modules/cloudSync') call in syncLibrary
(which gates on config.dbsync) to append a .catch(...) handler that logs the
error and optionally suppresses it (e.g., console.error or the project logger)
so failures loading the cloudSync module do not reject unhandled; keep the
existing .then(...) behavior that invokes addDirtyTrack, removeDirtyTrack, and
scheduleSync.

[coderabbitai]

Actionable comments posted: 3

🧹 Nitpick comments (1)

vite.config.ts (1)

7-10: Merge the two path imports.

resolve is imported from 'path' on Line 7 and path is again imported as a default on Line 9. Consolidate into a single statement.

♻️ Proposed fix

-import { resolve } from 'path';
-import { readdirSync } from 'fs';
-import path from 'path';
-import url from 'url';
+import path, { resolve } from 'path';
+import { readdirSync } from 'fs';

(The url import is addressed in a separate comment below.)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@vite.config.ts` around lines 7 - 10, You have two separate imports from
'path' (a named import resolve and a default import path); consolidate them by
removing the named import and keeping a single import (import path from 'path')
and update all uses of resolve(...) to path.resolve(...); look for the named
symbol resolve and the default path import in the module and replace calls to
resolve with path.resolve to use the single consolidated import.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@vite.config.ts`:
- Line 154: The current return of adapter(req, res) bypasses the surrounding
try/catch because the adapter's Promise isn't awaited; change the call in the
wrapper (where adapter, req, res, and next are used) to await the adapter (or
replace with a .then().catch() that calls next on error) so any rejection is
caught and routed to next(err) in the existing catch block; ensure you update
the return site that currently says return adapter(req, res) to use await (or a
promise chain) so adapter rejections are handled by the try/catch.
- Around line 142-164: The middleware implementation for apiMiddleware (in
configureServer -> server.middlewares.use) uses an async function and try/catch;
refactor it to use Promise chains instead: remove the async keyword on the
middleware callback and replace await
server.ssrLoadModule('./src/backend/localAdapter.ts') and await
server.ssrLoadModule(`./api/${endpoint}.ts`) with .then(...) chains, handling
errors with .catch(...) to call next() and logging the error; ensure you still
return adapter(req, res) from the successful promise chain and call next() for
non-/api/ requests and on errors so control flow remains identical to the
original.
- Line 10: Remove the deprecated import "import url from 'url';" and replace
uses of url.parse(req.url).pathname with the WHATWG URL API: construct a new URL
using the request URL and a base (e.g., new URL(req.url,
`http://${req.headers.host}`)) and read its .pathname; update all occurrences
(including where url.parse is used around lines 148-149) to use this pattern and
then delete the unused import. Ensure you preserve the same pathname behavior
and handle possible undefined req.url consistently.

---

Nitpick comments:
In `@vite.config.ts`:
- Around line 7-10: You have two separate imports from 'path' (a named import
resolve and a default import path); consolidate them by removing the named
import and keeping a single import (import path from 'path') and update all uses
of resolve(...) to path.resolve(...); look for the named symbol resolve and the
default path import in the module and replace calls to resolve with path.resolve
to use the single consolidated import.

[coderabbitai]

🛠️ Refactor suggestion | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

# First, locate and examine the vite.config.ts file
find . -name "vite.config.ts" -type f | head -5

Repository: n-ce/ytify

Length of output: 70

---

🏁 Script executed:

# Get file stats to check size
wc -l vite.config.ts 2>/dev/null || echo "File not found in root"

Repository: n-ce/ytify

Length of output: 72

---

🏁 Script executed:

# Try searching from repo root
fd "vite.config.ts" --type f

Repository: n-ce/ytify

Length of output: 68

---

🏁 Script executed:

cat -n vite.config.ts

Repository: n-ce/ytify

Length of output: 5729

---

🌐 Web query:

Node.js url.parse deprecated DEP0169 WHATWG URL API

💡 Result:

DEP0169 is Node.js’s deprecation for “Insecure url.parse()”: the legacy url.parse() behavior is not standardized, can be error-prone with security implications, and Node recommends migrating to the WHATWG URL API instead. In modern Node versions it’s an application deprecation (warnings for non-node_modules code). [1]

Replace url.parse() with WHATWG URL

// Legacy (deprecated)
const url = require('node:url');
const u = url.parse('https://user:pass@ex.com:8080/p?a=1#h');

// WHATWG (recommended)
const { URL } = require('node:url');
const u2 = new URL('https://user:pass@ex.com:8080/p?a=1#h');

console.log(u2.protocol);              // 'https:'
console.log(u2.username);              // 'user'
console.log(u2.password);              // 'pass'
console.log(u2.hostname);              // 'ex.com'
console.log(u2.port);                  // '8080'
console.log(u2.pathname);              // '/p'
console.log(u2.searchParams.get('a')); // '1'
console.log(u2.hash);                  // '#h'

Node’s node:url docs describe both the legacy API and the WHATWG URL API and show how their fields differ. [2]

Notes / common migrations

• url.parse(str, true) (object query) → new URL(str).searchParams (use get(), getAll(), entries(), or convert to an object yourself). [2]
• url.format(string) is also deprecated when you pass a string, because it calls url.parse() internally; use new URL(str).toString() to canonicalize/serialize. [2]
• If you need to parse relative URLs, use the base parameter: new URL(relative, base). [2]

Sources: Node.js deprecations list (DEP0169) [1], Node.js url / WHATWG URL API docs [2].

---

Replace deprecated url.parse() with the WHATWG URL API.

url.parse() is deprecated (DEP0169)—Node.js docs recommend using the WHATWG URL API instead. Since only pathname is consumed, the entire url module import can be removed.

Refactor suggestion

-import url from 'url';

-        const parsedUrl = url.parse(req.url, true);
-        const endpoint = parsedUrl.pathname?.split('/').pop();
+        const { pathname } = new URL(req.url!, 'http://localhost');
+        const endpoint = pathname.split('/').pop();

Also applies to: 148-149

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@vite.config.ts` at line 10, Remove the deprecated import "import url from
'url';" and replace uses of url.parse(req.url).pathname with the WHATWG URL API:
construct a new URL using the request URL and a base (e.g., new URL(req.url,
`http://${req.headers.host}`)) and read its .pathname; update all occurrences
(including where url.parse is used around lines 148-149) to use this pattern and
then delete the unused import. Ensure you preserve the same pathname behavior
and handle possible undefined req.url consistently.

[coderabbitai]

🛠️ Refactor suggestion | 🟠 Major

async/await + try/catch in middleware violates project coding guidelines.

The middleware at Lines 145–162 uses both async/await and try/catch, which the project guideline explicitly disallows in favour of Promise-chain .then().catch() style.

♻️ Proposed fix

 const apiMiddleware = (serve: boolean): PluginOption => serve ? {
   name: 'api-middleware',
   configureServer(server) {
-    server.middlewares.use(async (req, res, next) => {
-      if (req.url?.startsWith('/api/')) {
-        const { createLocalAdapter } = await server.ssrLoadModule('./src/backend/localAdapter.ts');
-        const parsedUrl = url.parse(req.url, true);
-        const endpoint = parsedUrl.pathname?.split('/').pop();
-
-        try {
-          const handlerModule = await server.ssrLoadModule(`./api/${endpoint}.ts`);
-          const adapter = createLocalAdapter(handlerModule.default);
-          return adapter(req, res);
-        } catch (e) {
-          console.error(`Failed to load API handler for ${endpoint}:`, e);
-          next();
-        }
-      } else {
-        next();
-      }
-    });
+    server.middlewares.use((req, res, next) => {
+      if (!req.url?.startsWith('/api/')) return next();
+
+      const { pathname } = new URL(req.url, 'http://localhost');
+      const endpoint = pathname.split('/').pop();
+
+      server.ssrLoadModule('./src/backend/localAdapter.ts')
+        .then(({ createLocalAdapter }) =>
+          server.ssrLoadModule(`./api/${endpoint}.ts`)
+            .then((handlerModule) => createLocalAdapter(handlerModule.default)(req, res))
+        )
+        .catch((e) => {
+          console.error(`Failed to load API handler for ${endpoint}:`, e);
+          next();
+        });
+    });
   }
 } : [];

As per coding guidelines: "Prefer Promise-based .then().catch() syntax over async/await or try...catch blocks".

📝 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.

Suggested change

	const apiMiddleware = (serve: boolean): PluginOption => serve ? {
	name: 'api-middleware',
	configureServer(server) {
	server.middlewares.use(async (req, res, next) => {
	if (req.url?.startsWith('/api/')) {
	const { createLocalAdapter } = await server.ssrLoadModule('./src/backend/localAdapter.ts');
	const parsedUrl = url.parse(req.url, true);
	const endpoint = parsedUrl.pathname?.split('/').pop();
	try {
	const handlerModule = await server.ssrLoadModule(`./api/${endpoint}.ts`);
	const adapter = createLocalAdapter(handlerModule.default);
	return adapter(req, res);
	} catch (e) {
	console.error(`Failed to load API handler for ${endpoint}:`, e);
	next();
	}
	} else {
	next();
	}
	});
	}
	} : [];
	const apiMiddleware = (serve: boolean): PluginOption => serve ? {
	name: 'api-middleware',
	configureServer(server) {
	server.middlewares.use((req, res, next) => {
	if (!req.url?.startsWith('/api/')) return next();
	const { pathname } = new URL(req.url, 'http://localhost');
	const endpoint = pathname.split('/').pop();
	server.ssrLoadModule('./src/backend/localAdapter.ts')
	.then(({ createLocalAdapter }) =>
	server.ssrLoadModule(`./api/${endpoint}.ts`)
	.then((handlerModule) => createLocalAdapter(handlerModule.default)(req, res))
	)
	.catch((e) => {
	console.error(`Failed to load API handler for ${endpoint}:`, e);
	next();
	});
	});
	}
	} : [];

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@vite.config.ts` around lines 142 - 164, The middleware implementation for
apiMiddleware (in configureServer -> server.middlewares.use) uses an async
function and try/catch; refactor it to use Promise chains instead: remove the
async keyword on the middleware callback and replace await
server.ssrLoadModule('./src/backend/localAdapter.ts') and await
server.ssrLoadModule(`./api/${endpoint}.ts`) with .then(...) chains, handling
errors with .catch(...) to call next() and logging the error; ensure you still
return adapter(req, res) from the successful promise chain and call next() for
non-/api/ requests and on errors so control flow remains identical to the
original.

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

adapter(req, res) rejection bypasses the catch block.

Because adapter(req, res) is not awaited — it is simply return-ed — any Promise rejection it emits is not caught by the surrounding try/catch. Errors from the adapter would surface as unhandled promise rejections rather than being routed through the next() fallback. The localAdapter description notes it handles errors internally, but this is not guaranteed for all runtime paths.

Change return adapter(req, res) to return await adapter(req, res) (or apply the .then().catch() refactor above, which avoids the issue entirely).

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@vite.config.ts` at line 154, The current return of adapter(req, res) bypasses
the surrounding try/catch because the adapter's Promise isn't awaited; change
the call in the wrapper (where adapter, req, res, and next are used) to await
the adapter (or replace with a .then().catch() that calls next on error) so any
rejection is caught and routed to next(err) in the existing catch block; ensure
you update the return site that currently says return adapter(req, res) to use
await (or a promise chain) so adapter rejections are handled by the try/catch.