feat: native TS parser, drop .py bridge 🐍🌉, restruc __tests__ - #83
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
Reviewer's GuideRewrites the YouTube Music integration to a native TypeScript pipeline with structured result types and server-side caching, introduces a diagnostics page and updated activity aggregation, and adds media-session persistence and accessibility improvements across several miscellaneous tools while simplifying SEO and package FAQs. Sequence diagram for YT Music recent tracks fetch and cachingsequenceDiagram
participant Api as api_ytmusic_recent_GET
participant Tracks as getYTMusicResult
participant Cache as readYTMusicCache/writeYTMusicCache
participant Auth as fetchInnertube
participant Parser as parser_ts
Api->>Tracks: getYTMusicResult(limit, GetYTMusicOptions)
Tracks->>Cache: readYTMusicCache()
Cache-->>Tracks: YTMusicCacheEntry | null
alt [cache is fresh and has tracks]
Tracks-->>Api: YTMusicResult(status ok, source database)
else [need live YouTube Music fetch]
Tracks->>Auth: fetchInnertube("browse", {browseId: "FEmusic_history"})
Auth-->>Tracks: innertubeResponse
Tracks->>Parser: parseInnertubeTracks(innertubeResponse, limit)
Parser-->>Tracks: YTMusicTrack[]
Tracks->>Parser: stabilizeTrackTimestamps(freshTracks, cachedTracks)
Parser-->>Tracks: YTMusicTrack[]
Tracks->>Cache: writeYTMusicCache(tracks)
Cache-->>Tracks: updatedAt
Tracks-->>Api: YTMusicResult(status ok | empty | unauthorized | error, source youtube-music)
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Contributor
|
Warning Review limit reached
Next review available in: 33 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (12)
📝 WalkthroughWalkthroughChangesThe PR adds a structured YouTube Music diagnostics and retrieval flow, IndexedDB persistence for media tools, activity accessibility improvements, FAQ and recent-tool removal, and broad Vitest coverage across routes and utilities. YouTube Music
Media tools
Accessibility and cleanup
Test coverage and content
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
remcostoeten
changed the title
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- In the
/api/ytmusic/recentroute, theunstable_rethrow(error)inside the catch block makes the subsequent logging and JSON error response unreachable; either drop the rethrow or handle rethrow-only cases separately so the control flow is clear. - The IndexedDB helpers in
media-persistence.tsassumewindow.indexedDBis always available; adding a feature check and short-circuiting when IndexedDB is unavailable would avoid runtime errors in unsupported environments.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In the `/api/ytmusic/recent` route, the `unstable_rethrow(error)` inside the catch block makes the subsequent logging and JSON error response unreachable; either drop the rethrow or handle rethrow-only cases separately so the control flow is clear.
- The IndexedDB helpers in `media-persistence.ts` assume `window.indexedDB` is always available; adding a feature check and short-circuiting when IndexedDB is unavailable would avoid runtime errors in unsupported environments.
## Individual Comments
### Comment 1
<location path="src/app/api/ytmusic/recent/route.ts" line_range="2" />
<code_context>
import { NextResponse } from 'next/server'
-import { getYTMusicTracks } from '@/server/ytmusic/tracks'
-import { hasYTMusicCredentials } from '@/server/ytmusic/auth'
+import { unstable_rethrow } from 'next/navigation'
+import { getYTMusicResult } from '@/server/ytmusic/tracks'
import { parseBoundedIntParam } from '@/shared/lib/request-params'
</code_context>
<issue_to_address>
**issue (bug_risk):** Using `unstable_rethrow` and returning a custom error response in the same catch block is contradictory.
In this handler `unstable_rethrow(error)` exits the catch block immediately, so the `console.error` and `NextResponse.json(...)` never run. This creates misleading control flow and may result in the error being handled/logged twice by different layers. Please either remove `unstable_rethrow` and rely on the explicit 500 response, or drop the custom JSON response and let the framework handle the rethrown error.
</issue_to_address>
### Comment 2
<location path="src/features/miscellaneous/utils/media-persistence.ts" line_range="33-35" />
<code_context>
+ return `${toolKey}:output`
+}
+
+function openDatabase(): Promise<IDBDatabase> {
+ return new Promise((resolve, reject) => {
+ const request = window.indexedDB.open(DB_NAME, DB_VERSION)
+ request.onupgradeneeded = () => {
+ if (!request.result.objectStoreNames.contains(STORE_NAME)) {
</code_context>
<issue_to_address>
**issue:** Directly using `window.indexedDB` without capability checks can throw in unsupported or restricted environments.
These helpers assume `window.indexedDB` always exists, but in some environments (e.g. older Safari, private browsing, hardened contexts) it can be missing or fail synchronously. A direct `window.indexedDB.open(...)` will then throw before any promise rejection handling. Please add a capability check (e.g. `if (!('indexedDB' in window)) { /* no-op persistence */ }`) so the UI keeps working when IndexedDB isn’t available.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 14
🧹 Nitpick comments (8)
src/features/miscellaneous/utils/media-persistence.ts (1)
97-106: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winRead all three keys in one transaction.
loadMediaSessioncallsreadKeythree times. Each call opens and closes a separateIDBDatabaseconnection. Use one connection and one read transaction instead. This reduces open/close churn on the restore path and gives a consistent snapshot of the three keys.♻️ Proposed refactor
+async function readKeys<T extends unknown[]>(keys: string[]): Promise<unknown[]> { + const db = await openDatabase() + try { + return await new Promise<unknown[]>((resolve, reject) => { + const store = db + .transaction(STORE_NAME) + .objectStore(STORE_NAME) + const requests = keys.map(key => store.get(key)) + const transaction = store.transaction + transaction.oncomplete = () => + resolve(requests.map(request => request.result)) + transaction.onerror = () => reject(transaction.error) + transaction.onabort = () => reject(transaction.error) + }) + } finally { + db.close() + } +} + export async function loadMediaSession( toolKey: string ): Promise<TPersistedSession> { - const [file, trim, output] = await Promise.all([ - readKey<File>(inputKey(toolKey)), - readKey<TTrimRange>(trimKey(toolKey)), - readKey<TPersistedOutput>(outputKey(toolKey)) - ]) + const [file, trim, output] = (await readKeys([ + inputKey(toolKey), + trimKey(toolKey), + outputKey(toolKey) + ])) as [File?, TTrimRange?, TPersistedOutput?] return { file: file ?? null, trim: trim ?? null, output: output ?? null } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/features/miscellaneous/utils/media-persistence.ts` around lines 97 - 106, Update loadMediaSession to open a single IndexedDB connection and execute one read-only transaction covering inputKey(toolKey), trimKey(toolKey), and outputKey(toolKey), rather than invoking readKey separately for each key. Reuse the transaction’s object store reads to build the existing { file, trim, output } result with null fallbacks, preserving the current return shape.src/features/miscellaneous/sendable-video/hooks/use-sendable-video-store.ts (1)
71-95: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffExtract the duplicated restore and trim-persistence logic. Three stores repeat the same block: a
restoredRefguard,setFileplusURL.createObjectURL, output reconstruction fromsession.output, and the identical "Restored your previous session from this browser." status. Two of them also repeat the same 300 ms debouncedpersistTrimeffect. Move the shared parts intouseMediaSessionor a small helper so a future change to the restore contract needs one edit.
src/features/miscellaneous/sendable-video/hooks/use-sendable-video-store.ts#L71-L95: move the guard, file/URL restore, and status message into the shared helper, and keep only thekindmapping local.src/features/miscellaneous/video-to-gif/hooks/use-video-to-gif-store.ts#L74-L98: use the shared helper, and move the debouncedpersistTrimeffect (lines 185-191) into a shareduseTrimPersistencehook.src/features/miscellaneous/gif-to-video/hooks/use-gif-to-video-store.ts#L62-L84: use the shared helper; this store needs no trim handling.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/features/miscellaneous/sendable-video/hooks/use-sendable-video-store.ts` around lines 71 - 95, Extract the duplicated session restoration into a shared helper or useMediaSession, including the restoredRef guard, file and object URL restoration, output reconstruction, and restored-session status; retain only each store’s local output kind mapping. In src/features/miscellaneous/sendable-video/hooks/use-sendable-video-store.ts lines 71-95, adopt the helper. In src/features/miscellaneous/video-to-gif/hooks/use-video-to-gif-store.ts lines 74-98, adopt it and move the 300 ms persistTrim effect from lines 185-191 into a shared useTrimPersistence hook. In src/features/miscellaneous/gif-to-video/hooks/use-gif-to-video-store.ts lines 62-84, adopt the helper without adding trim handling.__tests__/components/projects/server/github.test.ts (1)
10-20: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winRestore environment and global stubs after each test.
beforeEachclears state only before a test starts. If Vitest shares globals across files, the final test leavesNEXT_PHASEandfetchstubbed. AddafterEachcleanup withvi.unstubAllEnvs()andvi.unstubAllGlobals().🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@__tests__/components/projects/server/github.test.ts` around lines 10 - 20, Add an afterEach hook in the GitHub component test setup that calls vi.unstubAllEnvs() and vi.unstubAllGlobals() to restore environment variables and globals after every test; keep the existing beforeEach reset behavior unchanged.src/server/ytmusic/auth.ts (3)
46-49: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winWiden the config value unescaping.
readConfigValueonly converts\u003d. YouTube inlines other escape sequences in these JSON string literals, for example\u0026and\/.VISITOR_DATAis the most likely value to carry them, and a wrong visitor ID is passed silently to both the payload and theX-Goog-Visitor-Idheader.Decode the captured value as a JSON string instead of a single hand-rolled replacement.
♻️ Proposed refactor
function readConfigValue(html: string, key: string): string { - const match = html.match(new RegExp(`"${key}"\\s*:\\s*"([^"]+)"`)) - return match?.[1]?.replace(/\\u003d/gi, '=') ?? '' + const match = html.match(new RegExp(`"${key}"\\s*:\\s*"([^"]*)"`)) + if (!match?.[1]) return '' + try { + return JSON.parse(`"${match[1]}"`) as string + } catch { + return match[1] + } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/server/ytmusic/auth.ts` around lines 46 - 49, Update readConfigValue to decode the captured config value using JSON string parsing, so all standard JSON escapes such as \u003d, \u0026, and escaped slashes are handled instead of only replacing equals signs. Preserve the existing empty-string fallback when no matching value is found.
99-120: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMake
generateAuthHeadersynchronous.The function contains no
await. It performs only cookie lookups and SHA-1 hashing. Declare it as a plain function and drop theawaitat the call site on Line 145.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/server/ytmusic/auth.ts` around lines 99 - 120, Make generateAuthHeader a synchronous function by removing async, and remove the corresponding await from its call site around line 145. Preserve the existing cookie validation, hash generation, and returned authorization header unchanged.
135-142: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winDeduplicate concurrent bootstrap requests.
fetchPageConfigis awaited without a shared in-flight promise. The diagnostics page and the/api/ytmusic/recentroute can run at the same time, so several concurrent calls each fetchhttps://music.youtube.comand each overwritescachedPageConfig. This multiplies outbound bootstrap requests and increases the risk of throttling by YouTube.Store the pending promise and reuse it.
♻️ Proposed refactor
let cachedPageConfig: PageConfig | null = null +let pendingPageConfig: Promise<PageConfig> | null = nullif (!cachedPageConfig) { - cachedPageConfig = await fetchPageConfig(cachedCookieMap) + pendingPageConfig ??= fetchPageConfig(cachedCookieMap).finally(() => { + pendingPageConfig = null + }) + cachedPageConfig = await pendingPageConfig }Also clear
pendingPageConfiginsideinvalidateYTMusicClient.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/server/ytmusic/auth.ts` around lines 135 - 142, Deduplicate concurrent page-config bootstrap calls in the authentication flow by introducing a shared pending promise for fetchPageConfig, reusing it whenever cachedPageConfig is absent, and assigning the resolved result to the cache without starting duplicate requests. Update invalidateYTMusicClient to clear pendingPageConfig alongside the other cached client state.src/server/ytmusic/tracks.ts (1)
152-153: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the identity wrapper.
createResultreturns its argument unchanged. It only adds a call layer. TheYTMusicResulttype annotation on each object literal gives the same type checking.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/server/ytmusic/tracks.ts` around lines 152 - 153, Remove the redundant createResult function and its call sites in the result construction flow, returning the YTMusicResult object literals directly. Preserve the existing YTMusicResult annotations on those literals so type checking remains unchanged.src/app/api/ytmusic/recent/route.ts (1)
44-49: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winType the status parameter with the union.
getHttpStatusacceptsstring.YTMusicResult.statusis typed asYTMusicStatusinsrc/features/ytmusic/types.ts. With the wider parameter type, a misspelled literal in this function compiles and silently maps to 200. Use the union type and keep the mapping in a record so new statuses surface as type errors.♻️ Proposed refactor
+import type { YTMusicStatus } from '`@/features/ytmusic/types`'-function getHttpStatus(status: string): number { - if (status === 'unauthorized') return 401 - if (status === 'unconfigured') return 503 - if (status === 'error') return 502 - return 200 -} +const HTTP_STATUS_BY_RESULT: Record<YTMusicStatus, number> = { + ok: 200, + empty: 200, + stale: 200, + unauthorized: 401, + unconfigured: 503, + error: 502 +} + +function getHttpStatus(status: YTMusicStatus): number { + return HTTP_STATUS_BY_RESULT[status] ?? 200 +}Confirm the member list against
YTMusicStatus.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/api/ytmusic/recent/route.ts` around lines 44 - 49, Update getHttpStatus to accept the YTMusicStatus union instead of string, and replace the conditional mapping with a typed record that covers every YTMusicStatus member, preserving the existing HTTP status values and default behavior as appropriate.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@__tests__/app/api/spotify/auth-url/route.test.ts`:
- Around line 26-44: Update the test around the GET request and redirect_uri
assertion to stub SPOTIFY_REDIRECT_URI with a value different from the route’s
hardcoded fallback, then assert that distinct value is returned in
authUrl.searchParams. Keep the existing client ID and response checks unchanged.
In `@__tests__/app/api/spotify/now-playing/route.test.ts`:
- Around line 98-119: Update the fetch mock in the “retries once on 401 and
returns a non-playing state when spotify stays unavailable” test so both
responses explicitly represent failure, including setting the second response’s
ok value to false rather than relying on an omitted field. Keep the 401 retry
and non-playing response assertions unchanged.
In `@__tests__/server/ytmusic/tracks.test.ts`:
- Around line 56-58: Add an afterEach cleanup hook in the test suite containing
“does not call YouTube while the persistent cache is fresh” that calls
vi.useRealTimers(), ensuring fake timers and the mocked system time are restored
after every test.
In @.env.example:
- Around line 50-54: Remove the obsolete Python bridge scripts under
scripts/ytmusic/ if no runtime or build path still uses them, while preserving
the TypeScript server handling of YTM_AUTH_USER and YTM_COOKIE and the
corresponding environment validation and ignored generated auth/session files.
In
`@src/app/`(marketing)/blog/posts/yappin/stop-using-arrow-fnc-and-HUGE-REVEAL.md:
- Around line 27-32: Separate the two SomeView alternatives in the documentation
so they are not interpreted as duplicate declarations when copied. Use distinct
code fences or comment out one declaration, while preserving both alternative
implementations for readers.
- Around line 27-37: Update the SomeView example so it no longer returns an
arrow function: return the intended renderable view value directly, or revise
the surrounding prose if returning a function is intentional. Keep the
comparison and guidance consistent with the chosen behavior.
In `@src/components/landing/activity/contribution-graph.tsx`:
- Around line 434-460: Update handleGridKeyDown so ArrowDown and ArrowUp only
move within the active cell’s current week, preventing transitions between a
week’s last and first day. Preserve the existing clamping behavior for
horizontal navigation, Home, End, and valid vertical movement.
In `@src/features/miscellaneous/hooks/use-media-session.ts`:
- Around line 36-45: Update the useEffect restore flow around loadMediaSession
to track cancellation and return a cleanup that marks the effect inactive,
preventing onRestoreRef.current from running after unmount or toolKey changes.
Also replace the global startedRef guard with toolKey-aware tracking so each
dependency value can load once when toolKey changes.
In `@src/features/miscellaneous/sendable-video/utils/ffmpeg.ts`:
- Around line 23-27: Update isRemuxCompatible to inspect every audio stream in
probe rather than relying on probe.match’s first result. Reject remux
compatibility when any audio codec is not AAC or MP3, while preserving
compatibility for videos with no audio streams and for sources whose audio
streams are all supported.
In `@src/features/miscellaneous/utils/media-persistence.ts`:
- Around line 113-123: Update saveMediaFile to skip persistence when the input
file exceeds the configured MAX_INPUT_MB limit, preserving the existing clear
behavior for null files. Replace the separate deleteKeys and writeKey calls with
one readwrite transaction that removes trimKey(toolKey) and outputKey(toolKey)
and stores inputKey(toolKey) atomically, so a failed write cannot leave
inconsistent session data.
In `@src/features/ytmusic/components/ytmusic-diagnostics.tsx`:
- Around line 382-391: Update formatTimestamp to pass an explicit stable
timeZone, such as 'UTC', in the Intl.DateTimeFormat options so SSR and client
hydration produce identical timestamp text; preserve the existing invalid-date
handling and formatting fields.
In `@src/server/ytmusic/auth.ts`:
- Around line 194-196: Update the response failure handling in the YouTube Music
request flow to invalidate cachedPageConfig when authentication fails, ensuring
subsequent requests rebuild the bootstrap configuration. Import
YTMusicUnauthorizedError from parser and throw it specifically for 401 and 403
responses; preserve the existing generic error handling for other unsuccessful
statuses.
In `@src/server/ytmusic/cache.ts`:
- Around line 33-50: Update the getYTMusicResult flow to fetch and parse a fixed
maximum track window for writeYTMusicCache, rather than using the caller’s
limit. Persist that full window, then slice the cached or freshly parsed tracks
to the requested limit only when returning the result; keep writeYTMusicCache
unchanged as the persistence boundary.
In `@src/server/ytmusic/tracks.ts`:
- Around line 68-97: Parse and stabilize a fixed maximum history independently
of the request limit, then persist the complete stabilized collection via
writeYTMusicCache. Update the empty-result check to use allTracks, and return
only the requested limit from the response while retaining the full cache for
stale fallback and timestamp stabilization.
---
Nitpick comments:
In `@__tests__/components/projects/server/github.test.ts`:
- Around line 10-20: Add an afterEach hook in the GitHub component test setup
that calls vi.unstubAllEnvs() and vi.unstubAllGlobals() to restore environment
variables and globals after every test; keep the existing beforeEach reset
behavior unchanged.
In `@src/app/api/ytmusic/recent/route.ts`:
- Around line 44-49: Update getHttpStatus to accept the YTMusicStatus union
instead of string, and replace the conditional mapping with a typed record that
covers every YTMusicStatus member, preserving the existing HTTP status values
and default behavior as appropriate.
In `@src/features/miscellaneous/sendable-video/hooks/use-sendable-video-store.ts`:
- Around line 71-95: Extract the duplicated session restoration into a shared
helper or useMediaSession, including the restoredRef guard, file and object URL
restoration, output reconstruction, and restored-session status; retain only
each store’s local output kind mapping. In
src/features/miscellaneous/sendable-video/hooks/use-sendable-video-store.ts
lines 71-95, adopt the helper. In
src/features/miscellaneous/video-to-gif/hooks/use-video-to-gif-store.ts lines
74-98, adopt it and move the 300 ms persistTrim effect from lines 185-191 into a
shared useTrimPersistence hook. In
src/features/miscellaneous/gif-to-video/hooks/use-gif-to-video-store.ts lines
62-84, adopt the helper without adding trim handling.
In `@src/features/miscellaneous/utils/media-persistence.ts`:
- Around line 97-106: Update loadMediaSession to open a single IndexedDB
connection and execute one read-only transaction covering inputKey(toolKey),
trimKey(toolKey), and outputKey(toolKey), rather than invoking readKey
separately for each key. Reuse the transaction’s object store reads to build the
existing { file, trim, output } result with null fallbacks, preserving the
current return shape.
In `@src/server/ytmusic/auth.ts`:
- Around line 46-49: Update readConfigValue to decode the captured config value
using JSON string parsing, so all standard JSON escapes such as \u003d, \u0026,
and escaped slashes are handled instead of only replacing equals signs. Preserve
the existing empty-string fallback when no matching value is found.
- Around line 99-120: Make generateAuthHeader a synchronous function by removing
async, and remove the corresponding await from its call site around line 145.
Preserve the existing cookie validation, hash generation, and returned
authorization header unchanged.
- Around line 135-142: Deduplicate concurrent page-config bootstrap calls in the
authentication flow by introducing a shared pending promise for fetchPageConfig,
reusing it whenever cachedPageConfig is absent, and assigning the resolved
result to the cache without starting duplicate requests. Update
invalidateYTMusicClient to clear pendingPageConfig alongside the other cached
client state.
In `@src/server/ytmusic/tracks.ts`:
- Around line 152-153: Remove the redundant createResult function and its call
sites in the result construction flow, returning the YTMusicResult object
literals directly. Preserve the existing YTMusicResult annotations on those
literals so type checking remains unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 7ebe6e5d-6968-4a38-b8ad-40ad4643613e
📒 Files selected for processing (65)
.env.example__tests__/app/(marketing)/rss/route.test.ts__tests__/app/api/activity/combined/combine.test.ts__tests__/app/api/activity/combined/route.test.ts__tests__/app/api/github/contributions/route.test.ts__tests__/app/api/github/events/route.test.ts__tests__/app/api/spotify/auth-url/route.test.ts__tests__/app/api/spotify/callback/route.test.ts__tests__/app/api/spotify/dev-token/route.test.ts__tests__/app/api/spotify/now-playing/route.test.ts__tests__/app/api/spotify/recent/route.test.ts__tests__/app/api/sync/route.test.ts__tests__/app/api/ytmusic/recent/route.test.ts__tests__/components/landing/activity/activity-section-client.test.ts__tests__/components/projects/components/project-preview.test.tsx__tests__/components/projects/server/github.test.ts__tests__/components/providers/providers.test.ts__tests__/features/miscellaneous/diff-checker/utils/diff.test.ts__tests__/features/miscellaneous/find-replace/utils/search.test.ts__tests__/features/miscellaneous/find-replace/utils/text-transforms.test.ts__tests__/features/miscellaneous/json-tool/utils/json-tool.test.ts__tests__/features/miscellaneous/link-extractor/utils/link-extractor.test.ts__tests__/features/miscellaneous/svg-converter/utilities/svg-converter.test.ts__tests__/server/github/service.test.ts__tests__/server/spotify/tracks.test.ts__tests__/server/ytmusic/parser.test.ts__tests__/server/ytmusic/tracks.test.tsnext.config.tssrc/app/(marketing)/blog/posts/yappin/stop-using-arrow-fnc-and-HUGE-REVEAL.mdsrc/app/(marketing)/packages/[slug]/page.tsxsrc/app/(marketing)/ytmusic/page.tsxsrc/app/(tools)/tools/[slug]/page.tsxsrc/app/api/activity/combined/combine.tssrc/app/api/ytmusic/recent/route.tssrc/components/landing/activity/contribution-graph.tsxsrc/components/landing/activity/section.tsxsrc/features/miscellaneous/components/icons/animated-icons.csssrc/features/miscellaneous/components/media-dropzone.tsxsrc/features/miscellaneous/components/media-trim-panel.tsxsrc/features/miscellaneous/components/tool-quick-nav.tsxsrc/features/miscellaneous/components/tool-renderer.tsxsrc/features/miscellaneous/components/tool-seo-content.tsxsrc/features/miscellaneous/components/tools-hub.tsxsrc/features/miscellaneous/constants/tool-seo.tssrc/features/miscellaneous/gif-to-video/hooks/use-gif-to-video-store.tssrc/features/miscellaneous/hooks/use-media-session.tssrc/features/miscellaneous/hooks/use-tool-usage.tssrc/features/miscellaneous/hooks/use-trim-state.tssrc/features/miscellaneous/sendable-video/components/export-panel.tsxsrc/features/miscellaneous/sendable-video/hooks/use-sendable-video-store.tssrc/features/miscellaneous/sendable-video/index.tsxsrc/features/miscellaneous/sendable-video/utils/ffmpeg.tssrc/features/miscellaneous/utils/ffmpeg.tssrc/features/miscellaneous/utils/media-persistence.tssrc/features/miscellaneous/video-to-gif/hooks/use-video-to-gif-store.tssrc/features/packages/data.tssrc/features/ytmusic/components/ytmusic-diagnostics.tsxsrc/features/ytmusic/types.tssrc/server/env.tssrc/server/ytmusic/auth.tssrc/server/ytmusic/cache.tssrc/server/ytmusic/index.tssrc/server/ytmusic/parser.tssrc/server/ytmusic/python-bridge.tssrc/server/ytmusic/tracks.ts
💤 Files with no reviewable changes (7)
- src/features/miscellaneous/hooks/use-tool-usage.ts
- src/features/miscellaneous/components/tool-seo-content.tsx
- src/app/(tools)/tools/[slug]/page.tsx
- src/server/ytmusic/python-bridge.ts
- src/app/(marketing)/packages/[slug]/page.tsx
- src/features/miscellaneous/components/tool-renderer.tsx
- src/features/packages/data.ts
Comment on lines
+50
to
54
| # Browser profile index used by YouTube Music (usually 0). | ||
| YTM_AUTH_USER="0" | ||
| # Full Cookie request header copied from an authenticated music.youtube.com request. | ||
| # Keep this server-side and replace it when the diagnostic page reports unauthorized. | ||
| YTM_COOKIE="" |
Contributor
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check whether legacy Python YTM bridge scripts still exist and are referenced elsewhere.
fd . scripts/ytmusic
rg -n 'scripts/ytmusic|get_history\.py|patched_ytmusic\.py|server\.py' --type=json --type=md -g '!node_modules' -g '!scripts/ytmusic/**'Repository: remcostoeten/remcostoeten.nl
Length of output: 289
🏁 Script executed:
#!/bin/bash
# Description: Check repository references to the Python scripts and YTM scripts across tracked text files.
set -u
echo "== tracked files containing ytmusic references =="
git ls-files -z | tr '\0' '\n' | grep 'scripts/ytmusic' || true
echo
echo "== references to scripts/ytmusic or Python script names outside scripts/ytmusic =="
if command -v rg >/dev/null; then
rg -n --hidden --glob '!node_modules' --glob '!scripts/ytmusic/**' \
'scripts/ytmusic|get_history\.py|patched_ytmusic\.py|server\.py|setup_oauth\.py|YTM_AUTH_USER|YTM_COOKIE' . || true
else
echo "rg not available"
fi
echo
echo "== references to scripts/ytmusic or Python script names in git diff/stat =="
git diff --stat || true
git diff --name-only || true
git diff -- scripts | sed -n '1,220p' || trueRepository: remcostoeten/remcostoeten.nl
Length of output: 1467
Remove the legacy Python bridge scripts if they are no longer used.
YTM_AUTH_USER and YTM_COOKIE are now handled by the TypeScript server paths, while the only remaining references outside scripts/ytmusic/ are .env.example, environment validation, and generated auth/session JSON ignored by .gitignore. Delete scripts/ytmusic/*.py if this migration removes the Python bridge.
🧰 Tools
🪛 dotenv-linter (4.0.0)
[warning] 51-51: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
[warning] 54-54: [QuoteCharacter] The value has quote characters (', ")
(QuoteCharacter)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.env.example around lines 50 - 54, Remove the obsolete Python bridge scripts
under scripts/ytmusic/ if no runtime or build path still uses them, while
preserving the TypeScript server handling of YTM_AUTH_USER and YTM_COOKIE and
the corresponding environment validation and ignored generated auth/session
files.
Contributor
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 14
🧹 Nitpick comments (8)
src/features/miscellaneous/utils/media-persistence.ts (1)
97-106: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winRead all three keys in one transaction.
loadMediaSessioncallsreadKeythree times. Each call opens and closes a separateIDBDatabaseconnection. Use one connection and one read transaction instead. This reduces open/close churn on the restore path and gives a consistent snapshot of the three keys.♻️ Proposed refactor
+async function readKeys<T extends unknown[]>(keys: string[]): Promise<unknown[]> { + const db = await openDatabase() + try { + return await new Promise<unknown[]>((resolve, reject) => { + const store = db + .transaction(STORE_NAME) + .objectStore(STORE_NAME) + const requests = keys.map(key => store.get(key)) + const transaction = store.transaction + transaction.oncomplete = () => + resolve(requests.map(request => request.result)) + transaction.onerror = () => reject(transaction.error) + transaction.onabort = () => reject(transaction.error) + }) + } finally { + db.close() + } +} + export async function loadMediaSession( toolKey: string ): Promise<TPersistedSession> { - const [file, trim, output] = await Promise.all([ - readKey<File>(inputKey(toolKey)), - readKey<TTrimRange>(trimKey(toolKey)), - readKey<TPersistedOutput>(outputKey(toolKey)) - ]) + const [file, trim, output] = (await readKeys([ + inputKey(toolKey), + trimKey(toolKey), + outputKey(toolKey) + ])) as [File?, TTrimRange?, TPersistedOutput?] return { file: file ?? null, trim: trim ?? null, output: output ?? null } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/features/miscellaneous/utils/media-persistence.ts` around lines 97 - 106, Update loadMediaSession to open a single IndexedDB connection and execute one read-only transaction covering inputKey(toolKey), trimKey(toolKey), and outputKey(toolKey), rather than invoking readKey separately for each key. Reuse the transaction’s object store reads to build the existing { file, trim, output } result with null fallbacks, preserving the current return shape.src/features/miscellaneous/sendable-video/hooks/use-sendable-video-store.ts (1)
71-95: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚖️ Poor tradeoffExtract the duplicated restore and trim-persistence logic. Three stores repeat the same block: a
restoredRefguard,setFileplusURL.createObjectURL, output reconstruction fromsession.output, and the identical "Restored your previous session from this browser." status. Two of them also repeat the same 300 ms debouncedpersistTrimeffect. Move the shared parts intouseMediaSessionor a small helper so a future change to the restore contract needs one edit.
src/features/miscellaneous/sendable-video/hooks/use-sendable-video-store.ts#L71-L95: move the guard, file/URL restore, and status message into the shared helper, and keep only thekindmapping local.src/features/miscellaneous/video-to-gif/hooks/use-video-to-gif-store.ts#L74-L98: use the shared helper, and move the debouncedpersistTrimeffect (lines 185-191) into a shareduseTrimPersistencehook.src/features/miscellaneous/gif-to-video/hooks/use-gif-to-video-store.ts#L62-L84: use the shared helper; this store needs no trim handling.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/features/miscellaneous/sendable-video/hooks/use-sendable-video-store.ts` around lines 71 - 95, Extract the duplicated session restoration into a shared helper or useMediaSession, including the restoredRef guard, file and object URL restoration, output reconstruction, and restored-session status; retain only each store’s local output kind mapping. In src/features/miscellaneous/sendable-video/hooks/use-sendable-video-store.ts lines 71-95, adopt the helper. In src/features/miscellaneous/video-to-gif/hooks/use-video-to-gif-store.ts lines 74-98, adopt it and move the 300 ms persistTrim effect from lines 185-191 into a shared useTrimPersistence hook. In src/features/miscellaneous/gif-to-video/hooks/use-gif-to-video-store.ts lines 62-84, adopt the helper without adding trim handling.__tests__/components/projects/server/github.test.ts (1)
10-20: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winRestore environment and global stubs after each test.
beforeEachclears state only before a test starts. If Vitest shares globals across files, the final test leavesNEXT_PHASEandfetchstubbed. AddafterEachcleanup withvi.unstubAllEnvs()andvi.unstubAllGlobals().🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@__tests__/components/projects/server/github.test.ts` around lines 10 - 20, Add an afterEach hook in the GitHub component test setup that calls vi.unstubAllEnvs() and vi.unstubAllGlobals() to restore environment variables and globals after every test; keep the existing beforeEach reset behavior unchanged.src/server/ytmusic/auth.ts (3)
46-49: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winWiden the config value unescaping.
readConfigValueonly converts\u003d. YouTube inlines other escape sequences in these JSON string literals, for example\u0026and\/.VISITOR_DATAis the most likely value to carry them, and a wrong visitor ID is passed silently to both the payload and theX-Goog-Visitor-Idheader.Decode the captured value as a JSON string instead of a single hand-rolled replacement.
♻️ Proposed refactor
function readConfigValue(html: string, key: string): string { - const match = html.match(new RegExp(`"${key}"\\s*:\\s*"([^"]+)"`)) - return match?.[1]?.replace(/\\u003d/gi, '=') ?? '' + const match = html.match(new RegExp(`"${key}"\\s*:\\s*"([^"]*)"`)) + if (!match?.[1]) return '' + try { + return JSON.parse(`"${match[1]}"`) as string + } catch { + return match[1] + } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/server/ytmusic/auth.ts` around lines 46 - 49, Update readConfigValue to decode the captured config value using JSON string parsing, so all standard JSON escapes such as \u003d, \u0026, and escaped slashes are handled instead of only replacing equals signs. Preserve the existing empty-string fallback when no matching value is found.
99-120: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMake
generateAuthHeadersynchronous.The function contains no
await. It performs only cookie lookups and SHA-1 hashing. Declare it as a plain function and drop theawaitat the call site on Line 145.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/server/ytmusic/auth.ts` around lines 99 - 120, Make generateAuthHeader a synchronous function by removing async, and remove the corresponding await from its call site around line 145. Preserve the existing cookie validation, hash generation, and returned authorization header unchanged.
135-142: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winDeduplicate concurrent bootstrap requests.
fetchPageConfigis awaited without a shared in-flight promise. The diagnostics page and the/api/ytmusic/recentroute can run at the same time, so several concurrent calls each fetchhttps://music.youtube.comand each overwritescachedPageConfig. This multiplies outbound bootstrap requests and increases the risk of throttling by YouTube.Store the pending promise and reuse it.
♻️ Proposed refactor
let cachedPageConfig: PageConfig | null = null +let pendingPageConfig: Promise<PageConfig> | null = nullif (!cachedPageConfig) { - cachedPageConfig = await fetchPageConfig(cachedCookieMap) + pendingPageConfig ??= fetchPageConfig(cachedCookieMap).finally(() => { + pendingPageConfig = null + }) + cachedPageConfig = await pendingPageConfig }Also clear
pendingPageConfiginsideinvalidateYTMusicClient.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/server/ytmusic/auth.ts` around lines 135 - 142, Deduplicate concurrent page-config bootstrap calls in the authentication flow by introducing a shared pending promise for fetchPageConfig, reusing it whenever cachedPageConfig is absent, and assigning the resolved result to the cache without starting duplicate requests. Update invalidateYTMusicClient to clear pendingPageConfig alongside the other cached client state.src/server/ytmusic/tracks.ts (1)
152-153: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the identity wrapper.
createResultreturns its argument unchanged. It only adds a call layer. TheYTMusicResulttype annotation on each object literal gives the same type checking.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/server/ytmusic/tracks.ts` around lines 152 - 153, Remove the redundant createResult function and its call sites in the result construction flow, returning the YTMusicResult object literals directly. Preserve the existing YTMusicResult annotations on those literals so type checking remains unchanged.src/app/api/ytmusic/recent/route.ts (1)
44-49: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winType the status parameter with the union.
getHttpStatusacceptsstring.YTMusicResult.statusis typed asYTMusicStatusinsrc/features/ytmusic/types.ts. With the wider parameter type, a misspelled literal in this function compiles and silently maps to 200. Use the union type and keep the mapping in a record so new statuses surface as type errors.♻️ Proposed refactor
+import type { YTMusicStatus } from '`@/features/ytmusic/types`'-function getHttpStatus(status: string): number { - if (status === 'unauthorized') return 401 - if (status === 'unconfigured') return 503 - if (status === 'error') return 502 - return 200 -} +const HTTP_STATUS_BY_RESULT: Record<YTMusicStatus, number> = { + ok: 200, + empty: 200, + stale: 200, + unauthorized: 401, + unconfigured: 503, + error: 502 +} + +function getHttpStatus(status: YTMusicStatus): number { + return HTTP_STATUS_BY_RESULT[status] ?? 200 +}Confirm the member list against
YTMusicStatus.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/app/api/ytmusic/recent/route.ts` around lines 44 - 49, Update getHttpStatus to accept the YTMusicStatus union instead of string, and replace the conditional mapping with a typed record that covers every YTMusicStatus member, preserving the existing HTTP status values and default behavior as appropriate.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@__tests__/app/api/spotify/auth-url/route.test.ts`:
- Around line 26-44: Update the test around the GET request and redirect_uri
assertion to stub SPOTIFY_REDIRECT_URI with a value different from the route’s
hardcoded fallback, then assert that distinct value is returned in
authUrl.searchParams. Keep the existing client ID and response checks unchanged.
In `@__tests__/app/api/spotify/now-playing/route.test.ts`:
- Around line 98-119: Update the fetch mock in the “retries once on 401 and
returns a non-playing state when spotify stays unavailable” test so both
responses explicitly represent failure, including setting the second response’s
ok value to false rather than relying on an omitted field. Keep the 401 retry
and non-playing response assertions unchanged.
In `@__tests__/server/ytmusic/tracks.test.ts`:
- Around line 56-58: Add an afterEach cleanup hook in the test suite containing
“does not call YouTube while the persistent cache is fresh” that calls
vi.useRealTimers(), ensuring fake timers and the mocked system time are restored
after every test.
In @.env.example:
- Around line 50-54: Remove the obsolete Python bridge scripts under
scripts/ytmusic/ if no runtime or build path still uses them, while preserving
the TypeScript server handling of YTM_AUTH_USER and YTM_COOKIE and the
corresponding environment validation and ignored generated auth/session files.
In
`@src/app/`(marketing)/blog/posts/yappin/stop-using-arrow-fnc-and-HUGE-REVEAL.md:
- Around line 27-32: Separate the two SomeView alternatives in the documentation
so they are not interpreted as duplicate declarations when copied. Use distinct
code fences or comment out one declaration, while preserving both alternative
implementations for readers.
- Around line 27-37: Update the SomeView example so it no longer returns an
arrow function: return the intended renderable view value directly, or revise
the surrounding prose if returning a function is intentional. Keep the
comparison and guidance consistent with the chosen behavior.
In `@src/components/landing/activity/contribution-graph.tsx`:
- Around line 434-460: Update handleGridKeyDown so ArrowDown and ArrowUp only
move within the active cell’s current week, preventing transitions between a
week’s last and first day. Preserve the existing clamping behavior for
horizontal navigation, Home, End, and valid vertical movement.
In `@src/features/miscellaneous/hooks/use-media-session.ts`:
- Around line 36-45: Update the useEffect restore flow around loadMediaSession
to track cancellation and return a cleanup that marks the effect inactive,
preventing onRestoreRef.current from running after unmount or toolKey changes.
Also replace the global startedRef guard with toolKey-aware tracking so each
dependency value can load once when toolKey changes.
In `@src/features/miscellaneous/sendable-video/utils/ffmpeg.ts`:
- Around line 23-27: Update isRemuxCompatible to inspect every audio stream in
probe rather than relying on probe.match’s first result. Reject remux
compatibility when any audio codec is not AAC or MP3, while preserving
compatibility for videos with no audio streams and for sources whose audio
streams are all supported.
In `@src/features/miscellaneous/utils/media-persistence.ts`:
- Around line 113-123: Update saveMediaFile to skip persistence when the input
file exceeds the configured MAX_INPUT_MB limit, preserving the existing clear
behavior for null files. Replace the separate deleteKeys and writeKey calls with
one readwrite transaction that removes trimKey(toolKey) and outputKey(toolKey)
and stores inputKey(toolKey) atomically, so a failed write cannot leave
inconsistent session data.
In `@src/features/ytmusic/components/ytmusic-diagnostics.tsx`:
- Around line 382-391: Update formatTimestamp to pass an explicit stable
timeZone, such as 'UTC', in the Intl.DateTimeFormat options so SSR and client
hydration produce identical timestamp text; preserve the existing invalid-date
handling and formatting fields.
In `@src/server/ytmusic/auth.ts`:
- Around line 194-196: Update the response failure handling in the YouTube Music
request flow to invalidate cachedPageConfig when authentication fails, ensuring
subsequent requests rebuild the bootstrap configuration. Import
YTMusicUnauthorizedError from parser and throw it specifically for 401 and 403
responses; preserve the existing generic error handling for other unsuccessful
statuses.
In `@src/server/ytmusic/cache.ts`:
- Around line 33-50: Update the getYTMusicResult flow to fetch and parse a fixed
maximum track window for writeYTMusicCache, rather than using the caller’s
limit. Persist that full window, then slice the cached or freshly parsed tracks
to the requested limit only when returning the result; keep writeYTMusicCache
unchanged as the persistence boundary.
In `@src/server/ytmusic/tracks.ts`:
- Around line 68-97: Parse and stabilize a fixed maximum history independently
of the request limit, then persist the complete stabilized collection via
writeYTMusicCache. Update the empty-result check to use allTracks, and return
only the requested limit from the response while retaining the full cache for
stale fallback and timestamp stabilization.
---
Nitpick comments:
In `@__tests__/components/projects/server/github.test.ts`:
- Around line 10-20: Add an afterEach hook in the GitHub component test setup
that calls vi.unstubAllEnvs() and vi.unstubAllGlobals() to restore environment
variables and globals after every test; keep the existing beforeEach reset
behavior unchanged.
In `@src/app/api/ytmusic/recent/route.ts`:
- Around line 44-49: Update getHttpStatus to accept the YTMusicStatus union
instead of string, and replace the conditional mapping with a typed record that
covers every YTMusicStatus member, preserving the existing HTTP status values
and default behavior as appropriate.
In `@src/features/miscellaneous/sendable-video/hooks/use-sendable-video-store.ts`:
- Around line 71-95: Extract the duplicated session restoration into a shared
helper or useMediaSession, including the restoredRef guard, file and object URL
restoration, output reconstruction, and restored-session status; retain only
each store’s local output kind mapping. In
src/features/miscellaneous/sendable-video/hooks/use-sendable-video-store.ts
lines 71-95, adopt the helper. In
src/features/miscellaneous/video-to-gif/hooks/use-video-to-gif-store.ts lines
74-98, adopt it and move the 300 ms persistTrim effect from lines 185-191 into a
shared useTrimPersistence hook. In
src/features/miscellaneous/gif-to-video/hooks/use-gif-to-video-store.ts lines
62-84, adopt the helper without adding trim handling.
In `@src/features/miscellaneous/utils/media-persistence.ts`:
- Around line 97-106: Update loadMediaSession to open a single IndexedDB
connection and execute one read-only transaction covering inputKey(toolKey),
trimKey(toolKey), and outputKey(toolKey), rather than invoking readKey
separately for each key. Reuse the transaction’s object store reads to build the
existing { file, trim, output } result with null fallbacks, preserving the
current return shape.
In `@src/server/ytmusic/auth.ts`:
- Around line 46-49: Update readConfigValue to decode the captured config value
using JSON string parsing, so all standard JSON escapes such as \u003d, \u0026,
and escaped slashes are handled instead of only replacing equals signs. Preserve
the existing empty-string fallback when no matching value is found.
- Around line 99-120: Make generateAuthHeader a synchronous function by removing
async, and remove the corresponding await from its call site around line 145.
Preserve the existing cookie validation, hash generation, and returned
authorization header unchanged.
- Around line 135-142: Deduplicate concurrent page-config bootstrap calls in the
authentication flow by introducing a shared pending promise for fetchPageConfig,
reusing it whenever cachedPageConfig is absent, and assigning the resolved
result to the cache without starting duplicate requests. Update
invalidateYTMusicClient to clear pendingPageConfig alongside the other cached
client state.
In `@src/server/ytmusic/tracks.ts`:
- Around line 152-153: Remove the redundant createResult function and its call
sites in the result construction flow, returning the YTMusicResult object
literals directly. Preserve the existing YTMusicResult annotations on those
literals so type checking remains unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 7ebe6e5d-6968-4a38-b8ad-40ad4643613e
📒 Files selected for processing (65)
.env.example__tests__/app/(marketing)/rss/route.test.ts__tests__/app/api/activity/combined/combine.test.ts__tests__/app/api/activity/combined/route.test.ts__tests__/app/api/github/contributions/route.test.ts__tests__/app/api/github/events/route.test.ts__tests__/app/api/spotify/auth-url/route.test.ts__tests__/app/api/spotify/callback/route.test.ts__tests__/app/api/spotify/dev-token/route.test.ts__tests__/app/api/spotify/now-playing/route.test.ts__tests__/app/api/spotify/recent/route.test.ts__tests__/app/api/sync/route.test.ts__tests__/app/api/ytmusic/recent/route.test.ts__tests__/components/landing/activity/activity-section-client.test.ts__tests__/components/projects/components/project-preview.test.tsx__tests__/components/projects/server/github.test.ts__tests__/components/providers/providers.test.ts__tests__/features/miscellaneous/diff-checker/utils/diff.test.ts__tests__/features/miscellaneous/find-replace/utils/search.test.ts__tests__/features/miscellaneous/find-replace/utils/text-transforms.test.ts__tests__/features/miscellaneous/json-tool/utils/json-tool.test.ts__tests__/features/miscellaneous/link-extractor/utils/link-extractor.test.ts__tests__/features/miscellaneous/svg-converter/utilities/svg-converter.test.ts__tests__/server/github/service.test.ts__tests__/server/spotify/tracks.test.ts__tests__/server/ytmusic/parser.test.ts__tests__/server/ytmusic/tracks.test.tsnext.config.tssrc/app/(marketing)/blog/posts/yappin/stop-using-arrow-fnc-and-HUGE-REVEAL.mdsrc/app/(marketing)/packages/[slug]/page.tsxsrc/app/(marketing)/ytmusic/page.tsxsrc/app/(tools)/tools/[slug]/page.tsxsrc/app/api/activity/combined/combine.tssrc/app/api/ytmusic/recent/route.tssrc/components/landing/activity/contribution-graph.tsxsrc/components/landing/activity/section.tsxsrc/features/miscellaneous/components/icons/animated-icons.csssrc/features/miscellaneous/components/media-dropzone.tsxsrc/features/miscellaneous/components/media-trim-panel.tsxsrc/features/miscellaneous/components/tool-quick-nav.tsxsrc/features/miscellaneous/components/tool-renderer.tsxsrc/features/miscellaneous/components/tool-seo-content.tsxsrc/features/miscellaneous/components/tools-hub.tsxsrc/features/miscellaneous/constants/tool-seo.tssrc/features/miscellaneous/gif-to-video/hooks/use-gif-to-video-store.tssrc/features/miscellaneous/hooks/use-media-session.tssrc/features/miscellaneous/hooks/use-tool-usage.tssrc/features/miscellaneous/hooks/use-trim-state.tssrc/features/miscellaneous/sendable-video/components/export-panel.tsxsrc/features/miscellaneous/sendable-video/hooks/use-sendable-video-store.tssrc/features/miscellaneous/sendable-video/index.tsxsrc/features/miscellaneous/sendable-video/utils/ffmpeg.tssrc/features/miscellaneous/utils/ffmpeg.tssrc/features/miscellaneous/utils/media-persistence.tssrc/features/miscellaneous/video-to-gif/hooks/use-video-to-gif-store.tssrc/features/packages/data.tssrc/features/ytmusic/components/ytmusic-diagnostics.tsxsrc/features/ytmusic/types.tssrc/server/env.tssrc/server/ytmusic/auth.tssrc/server/ytmusic/cache.tssrc/server/ytmusic/index.tssrc/server/ytmusic/parser.tssrc/server/ytmusic/python-bridge.tssrc/server/ytmusic/tracks.ts
💤 Files with no reviewable changes (7)
- src/features/miscellaneous/hooks/use-tool-usage.ts
- src/features/miscellaneous/components/tool-seo-content.tsx
- src/app/(tools)/tools/[slug]/page.tsx
- src/server/ytmusic/python-bridge.ts
- src/app/(marketing)/packages/[slug]/page.tsx
- src/features/miscellaneous/components/tool-renderer.tsx
- src/features/packages/data.ts
🛑 Comments failed to post (2)
__tests__/app/api/spotify/auth-url/route.test.ts (1)
26-44: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Test does not distinguish configured value from the fallback.
SPOTIFY_REDIRECT_URIis stubbed to the same string as the route's hardcoded fallback ('http://127.0.0.1:3000/api/spotify/callback'). The assertion onredirect_uriat Line 42-44 passes even ifprocess.env.SPOTIFY_REDIRECT_URIis never read, because the fallback produces the identical value. Use a distinct URI for the stub to prove the environment variable actually flows through.🔧 Proposed fix
it('builds an auth url with the required scopes and configured redirect uri', async () => { vi.stubEnv('SPOTIFY_CLIENT_ID', 'client-123') vi.stubEnv( 'SPOTIFY_REDIRECT_URI', - 'http://127.0.0.1:3000/api/spotify/callback' + 'https://example.com/api/spotify/callback' ) const { GET } = await import('`@/app/api/spotify/auth-url/route`') const response = await GET() const data = await response.json() const authUrl = new URL(data.authUrl) expect(response.status).toBe(200) expect(authUrl.origin).toBe('https://accounts.spotify.com') expect(authUrl.pathname).toBe('/authorize') expect(authUrl.searchParams.get('client_id')).toBe('client-123') expect(authUrl.searchParams.get('redirect_uri')).toBe( - 'http://127.0.0.1:3000/api/spotify/callback' + 'https://example.com/api/spotify/callback' )📝 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.it('builds an auth url with the required scopes and configured redirect uri', async () => { vi.stubEnv('SPOTIFY_CLIENT_ID', 'client-123') vi.stubEnv( 'SPOTIFY_REDIRECT_URI', 'https://example.com/api/spotify/callback' ) const { GET } = await import('`@/app/api/spotify/auth-url/route`') const response = await GET() const data = await response.json() const authUrl = new URL(data.authUrl) expect(response.status).toBe(200) expect(authUrl.origin).toBe('https://accounts.spotify.com') expect(authUrl.pathname).toBe('/authorize') expect(authUrl.searchParams.get('client_id')).toBe('client-123') expect(authUrl.searchParams.get('redirect_uri')).toBe( 'https://example.com/api/spotify/callback' )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@__tests__/app/api/spotify/auth-url/route.test.ts` around lines 26 - 44, Update the test around the GET request and redirect_uri assertion to stub SPOTIFY_REDIRECT_URI with a value different from the route’s hardcoded fallback, then assert that distinct value is returned in authUrl.searchParams. Keep the existing client ID and response checks unchanged.__tests__/app/api/spotify/now-playing/route.test.ts (1)
98-119: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Retry-test mock does not represent "stays unavailable".
The second
fetchmock at Line 109 returns{ status: 204 }without an explicitokfield. Status 204 is a success status; a realfetchResponsewith status 204 hasok: true. The test only passes becauseokisundefinedhere, not because the mock represents an unavailable upstream, as the test name claims. Use a mock that reflects a genuine failure so the test verifies the intended scenario.🔧 Proposed fix
vi.stubGlobal( 'fetch', vi .fn() .mockResolvedValueOnce({ status: 401 }) - .mockResolvedValueOnce({ status: 204 }) + .mockResolvedValueOnce({ ok: false, status: 500 }) )📝 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.it('retries once on 401 and returns a non-playing state when spotify stays unavailable', async () => { authMocks.hasSpotifyCredentials.mockReturnValue(true) authMocks.getSpotifyAccessToken .mockResolvedValueOnce('token-1') .mockResolvedValueOnce('token-2') vi.stubGlobal( 'fetch', vi .fn() .mockResolvedValueOnce({ status: 401 }) .mockResolvedValueOnce({ ok: false, status: 500 }) ) const { GET } = await import('`@/app/api/spotify/now-playing/route`') const response = await GET() const data = await response.json() expect(authMocks.invalidateSpotifyTokenCache).toHaveBeenCalledTimes(1) expect(response.status).toBe(200) expect(data).toEqual({ isPlaying: false }) })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@__tests__/app/api/spotify/now-playing/route.test.ts` around lines 98 - 119, Update the fetch mock in the “retries once on 401 and returns a non-playing state when spotify stays unavailable” test so both responses explicitly represent failure, including setting the second response’s ok value to false rather than relying on an omitted field. Keep the 401 retry and non-playing response assertions unchanged.
- persist full 50-track window in ytmusic cache regardless of request limit - throw YTMusicUnauthorizedError on 401/403 and invalidate cached page config - guard media persistence behind indexedDB availability, atomic session writes, 150MB cap - cancel media session restore on unmount, drop StrictMode-breaking started guard - check every audio stream in isRemuxCompatible - pin diagnostics timestamps to UTC to avoid hydration mismatch - restore real timers after fake-timer test - keep contribution graph arrow navigation within the week - drop legacy python ytmusic bridge scripts
Owner
Author
![Screenshot of /packages/[slug]](https://80hoebpheaklxk73.public.blob.vercel-storage.com/remcostoeten-5cqngdnk9-remcostoetens-projects.vercel.app_packages_[slug].jpeg)
![Screenshot of /tools/[slug]](https://80hoebpheaklxk73.public.blob.vercel-storage.com/remcostoeten-5cqngdnk9-remcostoetens-projects.vercel.app_tools_[slug].jpeg)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Rewrites the YT Music integration as pure TypeScript and cleans up surrounding areas.
YT Music (main change)
python-bridge.ts) with a native TS parser(
src/server/ytmusic/parser.ts) — no more external process dependencysrc/server/ytmusic/cache.ts)/ytmusicdiagnostics page for inspecting the integrationauth.ts,tracks.ts)Tests
__tests__/directory intoapp/,components/,features/,server/mirroring the source tree (all detected as renames)
Media tools
use-media-sessionhook andmedia-persistenceutilityuse-tool-usagehookWhy
The Python bridge required a Python runtime on the host, which complicated deploys and was a
fragile external dependency. Parsing natively in TS removes that entirely and lets responses be
cached in-process.
Summary by Sourcery
Replace the YouTube Music Python bridge with a native TypeScript integration, add diagnostics and caching, and improve media tools persistence, accessibility, and SEO/content structure.
New Features:
Enhancements:
Build:
Documentation:
Tests:
Chores:
Summary by CodeRabbit
New Features
Improvements