Skip to content

feat: some improvements#225

Merged
roitium merged 15 commits into
devfrom
feat/1
Feb 24, 2026
Merged

feat: some improvements#225
roitium merged 15 commits into
devfrom
feat/1

Conversation

@roitium
Copy link
Copy Markdown
Collaborator

@roitium roitium commented Feb 23, 2026

Summary by CodeRabbit

  • 新功能

    • 本地歌单离线支持:离线时仅可播放已下载或已缓存歌曲;不可播放项灰显不可选
    • 本地歌单显示总时长;多选模式下支持拖拽重排并持久化顺序
    • 播放器控件改为 Lottie 动画并随主题着色;设置页等处加入底部播放栏
  • 修复

    • 修复离线启动导致歌单无限加载、播放器底部白块、桌面歌词卡死、短链接解析及多处崩溃问题
  • 改进

    • 全局本地封面缓存与封面同步下载/清理,优先使用本地封面提升离线体验
    • 弹幕与歌词在弱网/离线下更稳健并加入重试与网络感知策略

@roitium roitium added the enhancement New feature or request label Feb 23, 2026
@safedep
Copy link
Copy Markdown

safedep Bot commented Feb 23, 2026

SafeDep Report Summary

Green Malicious Packages Badge Green Vulnerable Packages Badge Green Risky License Badge

Package Details
Package Malware Vulnerability Risky License Report
icon fractional-indexing @ 3.2.0
apps/mobile/package.json pnpm-lock.yaml
ok icon
ok icon
ok icon
🔗
icon lottie-react-native @ 7.3.6
apps/mobile/package.json pnpm-lock.yaml
ok icon
ok icon
ok icon
🔗

This report is generated by SafeDep Github App

@vercel
Copy link
Copy Markdown

vercel Bot commented Feb 23, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
bbplayer-docs Ready Ready Preview, Comment Feb 24, 2026 1:02pm

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Feb 23, 2026

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

本次 PR 添加离线可播放判断与播放项灰显、在本地歌单中实现多选模式拖拽重排、引入 Lottie 播放控件与着色工具、重构 Orpheus 查询钩子并新增本地缓存查询 API,以及 Android 端封面缓存管理与若干 DB/迁移与工具函数更新。

Changes

Cohort / File(s) Summary
文档与变更日志
apps/docs/docs/guides/playlist.md, apps/mobile/CHANGELOG.md
新增本地歌单离线行为说明并更新 CHANGELOG。
依赖 & 资源
apps/mobile/package.json, apps/mobile/src/assets/lottie/play-pause.json, apps/mobile/src/assets/lottie/skip-next.json, apps/mobile/src/assets/lottie/skip-prev.json
新增 fractional-indexinglottie-react-native 依赖;新增三份 Lottie 动画资源。
Lottie 工具与 UI
apps/mobile/src/utils/lottie.ts, apps/mobile/src/features/player/components/PlayerControls.tsx
新增 tintLottieSource 着色工具;PlayerControls 替换为 Lottie + RectButton,加入去抖动播放/缓冲状态与动画帧控制。
Orpheus 查询层重构
apps/mobile/src/hooks/queries/orpheus/index.ts, apps/mobile/src/hooks/queries/bilibili/danmaku.ts, apps/mobile/src/hooks/queries/lyrics/index.ts
新增集中式 orpheus 查询模块及多个 use* 钩子与 queryKeys;为 bilibili 弹幕添加 segment 查询;歌词查询加入 networkMode 配置。
替换旧钩子/消费端改动
apps/mobile/src/hooks/player/useBatchDownloadStatus.ts (deleted), apps/mobile/src/components/modals/PlayerQueueModal.tsx, apps/mobile/src/components/modals/player/SleepTimerModal.tsx, apps/mobile/src/features/player/components/PlayerHeader.tsx, apps/mobile/src/features/player/components/PlayerFunctionalMenu.tsx
移除旧的 useBatchDownloadStatus 文件,多处组件改为使用新 queries/orpheus 导出的钩子(usePlayerQueue、useSleepTimerEndTime、useShuffleMode 等),并更新导入路径。
本地歌单页面与离线播放
apps/mobile/src/app/playlist/local/[id].tsx, apps/mobile/src/features/playlist/local/hooks/useLocalPlaylistPlayer.ts, apps/mobile/src/features/playlist/local/components/LocalPlaylistHeader.tsx, apps/mobile/src/features/playlist/local/components/LocalTrackList.tsx, apps/mobile/src/features/playlist/local/components/LocalPlaylistItem.tsx
加入网络感知、计算 playableOfflineKeys 并在离线时灰显/禁止不可用曲目;将 totalDuration 暴露到头部;扩展 useLocalPlaylistPlayer、LocalTrackList、PlaylistHeader 与 TrackItem 的接口以支持离线与拖拽回调。
本地歌单拖拽重排
apps/mobile/src/app/playlist/local/[id].tsx, apps/mobile/src/features/playlist/local/components/LocalTrackList.tsx, apps/mobile/src/features/playlist/local/components/LocalPlaylistItem.tsx, apps/mobile/src/hooks/mutations/db/playlist.ts
实现多选模式下拖拽重排(幽灵项、插入线、自动滚动、位置测量),新增 useReorderLocalPlaylistTrack mutation 并使用 fractional-indexing 生成/提交新的 sortKey。
后端服务 & DB:排序键迁移
apps/mobile/src/lib/services/playlistService.ts, apps/mobile/drizzle/0013_jittery_randall.sql, apps/mobile/drizzle/meta/0013_snapshot.json, apps/mobile/drizzle/meta/_journal.json, apps/mobile/drizzle/migrations.js, apps/mobile/src/lib/db/schema.ts, apps/mobile/src/hooks/app/useFastMigrations.ts, apps/mobile/drizzle/...
将本地歌单从数值 order 切换为文本型 sort_key(fractional indexing):SQL 迁移、Drizzle 快照更新、服务层与游标改为使用 sortKey,并在 JS 层加入 sort_key 迁移逻辑。
数据类型更新
apps/mobile/src/types/services/playlist.ts, apps/mobile/src/hooks/queries/db/playlist.ts, apps/mobile/src/types/storage.ts
重命名 ReorderLocalPlaylistTrackPayload,替换 fromOrder/toOrder 为 prevSortKey/nextSortKey;infinite hook 初始页参数改为 lastSortKey;在 storage schema 中加入 sort_key_migrated_v1 标志。
弹幕加载器增强
apps/mobile/src/features/player/hooks/danmaku/useDanmakuLoader.ts
增加离线短路、按段重试的指数退避与基于 fetchDanmakuSegmentQuery 的查询集成,改进重试与日志。
歌词与时长工具
apps/mobile/src/lib/services/lyricService.ts, apps/mobile/src/utils/time.ts
歌词网络路径在离线时短路返回 LyricNotFoundError;新增 formatDurationToText 用于显示总时长文案。
Android 原生 Orpheus 改动
packages/orpheus/android/src/main/java/.../CachedUriManager.kt, packages/orpheus/android/src/main/java/.../ExpoOrpheusModule.kt, packages/orpheus/android/src/main/java/.../GeneralStorage.kt, packages/orpheus/android/src/main/java/.../TrackRecordExtension.kt, packages/orpheus/src/ExpoOrpheusModule.ts, packages/orpheus/android/src/main/java/.../OrpheusMusicService.kt, packages/orpheus/android/src/main/java/.../GlideBitmapLoader.kt
新增 CachedUriManager 跟踪完全缓存 URI;toMediaItem 系列加入 context 参数并优先使用已下载封面;新增 getLruCachedUris API;调整 Glide 缓存策略与队列还原上下文传递。
其他小改动
apps/mobile/src/app/download.tsx, apps/mobile/src/components/modals/PlayerQueueModal.tsx, apps/mobile/src/components/modals/player/SleepTimerModal.tsx, apps/mobile/src/app/(tabs)/_layout.tsx, apps/mobile/src/app/_layout.tsx, apps/mobile/src/app/settings/..., apps/mobile/src/features/library/...
若干组件将直接 useQuery 替换为集中钩子(useDownloadTasks、usePlayerQueue、useSleepTimerEndTime);设置页拆分/路由调整;多个页面加入固定 NowPlayingBar 并调整底部内边距。

Sequence Diagram(s)

sequenceDiagram
    participant User as 用户
    participant UI as 本地歌单页面
    participant DB as 本地数据库
    participant Orpheus as Orpheus 原生层
    participant Cache as CachedUriManager

    User->>UI: 打开本地歌单
    UI->>UI: 检测网络状态 (useNetworkState)
    UI->>DB: 查询歌单与轨道(getPlaylistTracks/metadata)
    DB-->>UI: 返回曲目列表与 totalDuration
    UI->>Orpheus: 请求已缓存 URI / 批量下载状态 (useBatchDownloadStatus / getLruCachedUris)
    Orpheus->>Cache: 查询 URI 是否完全缓存
    Cache-->>Orpheus: 返回已缓存 URI 列表
    Orpheus-->>UI: 返回下载/缓存状态
    UI->>UI: 生成 playableOfflineKeys 并灰显不可用项
    User->>UI: 点击播放
    UI->>Orpheus: 请求播放(仅允许 playableOfflineKeys 中的项)
    Orpheus->>Cache: 读取本地媒体或标记未命中
    Cache-->>Orpheus: 提供本地 URI 或未命中
    Orpheus-->>User: 开始播放或返回错误
Loading
sequenceDiagram
    participant Dragger as 用户(拖拽)
    participant UI as LocalTrackList
    participant Mutation as useReorderLocalPlaylistTrack
    participant DB as PlaylistService

    Dragger->>UI: 长按并拖动项(onDragStart/onDragUpdate)
    UI->>UI: 渲染幽灵项,计算插入位置并自动滚动
    Dragger->>UI: 释放(onDragEnd)
    UI->>Mutation: 调用 reorderLocalPlaylistTrack(playlistId, trackId, prevSortKey, nextSortKey)
    Mutation->>DB: 执行后端重排(generateKeyBetween)
    DB-->>Mutation: 返回成功
    Mutation->>UI: 触发列表刷新并更新显示
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

Poem

🐰 我在草地里数歌单,轻轻拖动成新行,
Lottie 眨眼像星光,离线也能轻声唱,
缓存封面悄悄藏,原生问候带上下文,
兔兔跳过每一行,排序安放在月光旁,
祝播放顺利,曲曲皆成暖午眠。

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 10.81% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive 标题过于宽泛且缺乏具体信息,"some improvements"无法准确概括本次更改的主要内容。 建议更新标题为更具体的描述,如"feat: add offline playlist support and drag-and-drop reordering"或"feat: improve offline playback and local playlist management",以清晰反映本次PR的核心功能变更。
✅ 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 unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch feat/1

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

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

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

🧹 Nitpick comments (11)
packages/orpheus/android/src/main/java/expo/modules/orpheus/manager/CachedUriManager.kt (1)

73-74: 建议移除空的 onSpanTouched 方法体

detekt 警告此处为空函数块(EmptyFunctionBlock)。由于 onSpanTouched 仅表示某个 span 被"访问"(更新访问时间),无需处理,直接删除即可减少噪音。

♻️ 建议改动
-    override fun onSpanTouched(cache: Cache, oldSpan: CacheSpan, newSpan: CacheSpan) {
-    }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@packages/orpheus/android/src/main/java/expo/modules/orpheus/manager/CachedUriManager.kt`
around lines 73 - 74, 在 CachedUriManager 类中移除空的 override 方法 onSpanTouched(cache:
Cache, oldSpan: CacheSpan, newSpan:
CacheSpan);该方法体为空且无需处理访问事件,直接删除整个方法覆盖(保留类声明不变)以消除 detekt 的 EmptyFunctionBlock
警告;若接口要求实现且不能删除,则替换为明确的 `@Suppress`("EmptyFunctionBlock") 注解并保留方法签名以抑制警告。
packages/orpheus/android/src/main/java/expo/modules/orpheus/ExpoOrpheusModule.kt (1)

764-772: getLruCachedUris 异常被吞噬,缺少日志

捕获的异常未打印任何日志,若 CachedUriManager 未完成初始化或内部抛出异常,调用方会静默拿到空列表,无法排查问题(detekt SwallowedException 警告)。

♻️ 建议改动
         Function("getLruCachedUris") { uris: List<String> ->
             try {
                 uris.filter { uri -> 
                     expo.modules.orpheus.manager.CachedUriManager.isFullyCached(uri) 
                 }
             } catch (e: Exception) {
+                Log.e("Orpheus", "getLruCachedUris failed", e)
                 emptyList<String>()
             }
         }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@packages/orpheus/android/src/main/java/expo/modules/orpheus/ExpoOrpheusModule.kt`
around lines 764 - 772, The getLruCachedUris lambda currently swallows all
exceptions; modify it to catch the Exception from
expo.modules.orpheus.manager.CachedUriManager.isFullyCached and log the error
before returning an empty list so failures are visible; locate the
Function("getLruCachedUris") block and update its catch to call Android logging
(e.g., Log.e with a TAG) or the module logger and include the exception and
context (the uris or a short message) while preserving the emptyList<String>()
return behavior.
apps/mobile/src/hooks/queries/bilibili/danmaku.ts (1)

16-22: 建议为播放器场景显式配置 retry

queryClient.fetchQuery 默认继承 React Query 的 3 次重试策略(含指数退避),在弹幕加载场景中可能带来较长等待(最差情况约数十秒)。结合本 PR 新增的离线支持,弹幕加载失败时更希望快速降级而非多次重试。

♻️ 建议显式配置 retry
 	return queryClient.fetchQuery({
 		queryKey: danmakuQueryKeys.segment(bvid, cid, segmentIndex),
 		queryFn: () =>
 			returnOrThrowAsync(bilibiliApi.getSegDanmaku(bvid, cid, segmentIndex)),
 		staleTime: 1000 * 60 * 5,
 		gcTime: 1000 * 60 * 10,
+		retry: 1,
 	})
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/mobile/src/hooks/queries/bilibili/danmaku.ts` around lines 16 - 22, The
fetchQuery call for danmaku should explicitly set React Query's retry behavior
to avoid long exponential backoffs in the player; update the
queryClient.fetchQuery invocation (where you use danmakuQueryKeys.segment,
returnOrThrowAsync and bilibiliApi.getSegDanmaku) to include an options object
with retry: 0 (or a small integer) so failed danmaku fetches fall back quickly
instead of inheriting the default 3 retries.
apps/mobile/src/utils/time.ts (1)

37-44: text.trim() 实际上是空操作

两个分支中生成的字符串均不含前导或尾随空白(hours > 0 时是 "X 小时 Y 分钟"hours === 0 时是 "Y 分钟"),trim() 对最终结果无任何影响,可安全移除。

另外,当 hours > 0 && minutes === 0 时,会输出 "X 小时 0 分钟",如有意隐藏零分钟可选择过滤:

♻️ 建议移除无效 trim,并可选处理 0 分钟
 export const formatDurationToText = (seconds: number): string => {
 	const hours = Math.floor(seconds / 3600)
 	const minutes = Math.floor((seconds % 3600) / 60)
-	let text = ''
-	if (hours > 0) text += `${hours} 小时 `
-	text += `${minutes} 分钟`
-	return text.trim()
+	if (hours > 0 && minutes === 0) return `${hours} 小时`
+	if (hours > 0) return `${hours} 小时 ${minutes} 分钟`
+	return `${minutes} 分钟`
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/mobile/src/utils/time.ts` around lines 37 - 44, 在
formatDurationToText(seconds: number) 中移除无效的 text.trim()(生成结果在两个分支均无前后空白),并可选性地在
hours > 0 且 minutes === 0 的情况下不输出 "0 分钟":即构建字符串时只在 hours > 0 时追加 `${hours}
小时`,再在 minutes > 0 时追加 `${minutes} 分钟`,确保返回值不再调用 trim()。
apps/mobile/src/lib/services/lyricService.ts (1)

224-233: 建议将 fromSafePromise 替换为 fromPromise 以增强健壮性

ResultAsync.fromSafePromise 假设传入的 Promise 永不 reject。若 Network.getNetworkStateAsync() 在极端情况下(如原生模块未就绪、权限未授予)抛出异常,当前写法会产生未处理的 Promise rejection,导致整个 orElse 链挂起或崩溃。使用 fromPromise 可以将异常正确映射为 LyricNotFoundError

♻️ 建议修复
 		.orElse(() => {
-			return ResultAsync.fromSafePromise(
+			return ResultAsync.fromPromise(
 				Network.getNetworkStateAsync(),
+				() => new LyricNotFoundError('无法获取网络状态'),
 			).andThen((networkState) => {
 				if (networkState.isConnected === false) {
 					return errAsync(
 						new LyricNotFoundError('当前处于离线状态,无法获取网络歌词'),
 					)
 				}
 				return fetchFromNetwork()
 			})
 		})
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/mobile/src/lib/services/lyricService.ts` around lines 224 - 233, Replace
the use of ResultAsync.fromSafePromise with ResultAsync.fromPromise when
wrapping Network.getNetworkStateAsync() inside the orElse chain in
lyricService.ts so rejections are handled; ensure you map any thrown error from
Network.getNetworkStateAsync() into a retryable/handled failure (e.g., return
errAsync(new LyricNotFoundError(...)) or convert the rejection to the same error
path) and keep the subsequent logic that checks networkState.isConnected and
calls fetchFromNetwork() intact.
apps/mobile/src/hooks/queries/orpheus/index.ts (2)

57-60: usePlayerQueuequeryFn 中存在冗余的 async/await

Orpheus.getQueue() 已返回 Promise,中间变量 qawait 均无必要。

♻️ 简化 queryFn
-		queryFn: async () => {
-			const q = await Orpheus.getQueue()
-			return q
-		},
+		queryFn: () => Orpheus.getQueue(),
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/mobile/src/hooks/queries/orpheus/index.ts` around lines 57 - 60, In
usePlayerQueue's queryFn, remove the unnecessary async/await and intermediate
variable: instead of awaiting Orpheus.getQueue() into q and returning q, have
queryFn return the Promise from Orpheus.getQueue() directly (i.e., replace the
async function body so it simply returns Orpheus.getQueue()); this removes the
redundant async wrapper while keeping behavior identical. Reference:
usePlayerQueue -> queryFn -> Orpheus.getQueue.

44-52: useDownloadTasks 缺少显式 gcTime: 0,与其他钩子不一致

其他所有钩子(useShuffleModeuseBatchDownloadStatususePlayerQueueuseSleepTimerEndTime)均显式设置了 gcTime: 0,而 useDownloadTasks 仅依赖 setQueryDefaults 提供该值。功能上虽然正确,但在维护时容易产生困惑。

♻️ 建议补全 `gcTime`
 export function useDownloadTasks() {
 	return useQuery({
 		queryKey: orpheusQueryKeys.downloadTasks(),
 		queryFn: async () => {
 			return await Orpheus.getUncompletedDownloadTasks()
 		},
 		staleTime: 0,
+		gcTime: 0,
 	})
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/mobile/src/hooks/queries/orpheus/index.ts` around lines 44 - 52,
useDownloadTasks currently omits an explicit gcTime: 0 option, relying on
setQueryDefaults while other hooks set gcTime: 0 explicitly; update the
useDownloadTasks query options (in the useDownloadTasks function) to include
gcTime: 0 alongside staleTime: 0 so it matches the other hooks and avoids
confusion during maintenance.
apps/mobile/src/components/modals/PlayerQueueModal.tsx (1)

120-120: enabled 翻转已自动触发拉取,useEffect 中的显式 refetch() 冗余

usePlayerQueue(isVisible)isVisiblefalse 变为 true 时会因 enabled 翻转自动触发一次拉取(staleTime: 0)。紧接着第 168 行的 void refetch() 会再次触发,虽然 React Query 会对并发请求去重不产生实际双重请求,但逻辑上冗余。

可以移除 void refetch() 那一行,只保留 isVisiblefalse 时重置 didInitialScroll 的逻辑:

♻️ 去除冗余 refetch
 	// oxlint-disable-next-line react-you-might-not-need-an-effect/no-reset-all-state-on-prop-change
 	useEffect(() => {
-		if (isVisible) {
-			void refetch()
-		} else {
+		if (!isVisible) {
 			setDidInitialScroll(false)
 		}
-	}, [isVisible, refetch])
+	}, [isVisible])

Also applies to: 166-172

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/mobile/src/components/modals/PlayerQueueModal.tsx` at line 120, 在
useEffect 里移除冗余的 refetch 调用:usePlayerQueue(isVisible) 已经通过 enabled 翻转在 isVisible
从 false->true 时触发拉取(staleTime: 0),所以不要再显式调用 refetch();保留对 didInitialScroll
的重置逻辑(当 isVisible 为 false 时重置),更新或删除 useEffect 中对 refetch 的那一行(涉及符号
usePlayerQueue、refetch、isVisible、didInitialScroll、useEffect)以简化逻辑且避免冗余调用。
apps/mobile/src/features/player/components/PlayerControls.tsx (1)

123-123: 建议清理注释中的不当用语。

Line 123 的注释含有不太适合代码库的措辞。虽然诙谐,但长期维护角度建议替换为更中性的表述。

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/mobile/src/features/player/components/PlayerControls.tsx` at line 123,
Replace the inappropriate wording in the inline comment inside the
PlayerControls component: locate the comment containing "tmd" and change it to a
neutral, professional phrase (e.g., "temporary workaround" or "hack") such as
"Note: this is a temporary workaround; colorFilters do not take effect." Ensure
the comment remains concise and explanatory and update only the comment text in
PlayerControls.tsx.
apps/mobile/src/utils/lottie.ts (2)

14-16: 正则替换 [1,1,1,1] 存在误匹配风险(当前资源可控,但需注意)。

JSON.stringify 后全局替换 [1,1,1,1] 可能匹配到非颜色上下文中恰好为 [1,1,1,1] 的数组(如关键帧数据、变换值等)。当前的三个 Lottie 资源中不存在此问题,但如果未来新增的 Lottie 文件包含此类数据,替换会产生错误结果。

可以考虑添加注释说明此约束,或使用更精确的匹配(如 "k":[1,1,1,1])。

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/mobile/src/utils/lottie.ts` around lines 14 - 16, 当前实现对
JSON.stringify(source).replace(/\[1,1,1,1\]/g, ...) 全局替换可能误中非颜色数组;在
apps/mobile/src/utils/lottie.ts 中的返回语句(返回值类型
AnimationObject)请改为更精确的替换:要么在替换中加入键上下文如匹配 /"k"\s*:\s*\[1,1,1,1\]/
或类似只匹配色彩字段的正则,确保只替换颜色属性;更稳健的做法是遍历 source 对象并在遇到预期的 color 属性(或特定图层/属性名)时替换
[1,1,1,1],同时在代码中加注释说明该约束以避免未来误用。

10-13: 十六进制颜色解析缺少输入校验,可能导致 JSON.parse 抛出异常。

如果 hexColor 不是合法的 6 位十六进制字符串(例如短于 6 位、包含非法字符、或传入 3 位缩写格式),parseInt 会返回 NaN.toFixed(4) 产生字符串 "NaN",最终 JSON.parse 解析 [NaN,...] 时会抛出 SyntaxError

建议至少添加基本的格式断言或回退逻辑。

💡 建议的防御性校验
 export function tintLottieSource(
 	source: AnimationObject,
 	hexColor: string,
 ): AnimationObject {
 	const hex = hexColor.replace('#', '')
+	if (!/^[0-9a-fA-F]{6}$/.test(hex)) {
+		return source
+	}
 	const r = (parseInt(hex.slice(0, 2), 16) / 255).toFixed(4)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/mobile/src/utils/lottie.ts` around lines 10 - 13, The
hex-to-normalized-RGB conversion lacks input validation for hexColor; update the
code that computes hex, r, g, b to first normalize and validate hexColor (strip
leading '#', accept 3- or 6‑char forms by expanding 3->6, ensure
/^[0-9a-fA-F]{6}$/), and if validation fails use a safe fallback (e.g., default
"000000") or throw a clear error; after validation parseInt results check for
NaN before calling toFixed (or convert to numbers and then toFixed) so
JSON.parse will never receive "NaN" strings. Use the existing hexColor variable
and the hex/r/g/b computation block as the insertion point.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@apps/mobile/src/app/playlist/local/`[id].tsx:
- Around line 104-133: playAll currently filters the full playlist using
playableOfflineKeys (which is derived from allLoadedTracks/page-only data),
causing cached/downloaded tracks in unloaded pages to be excluded; inside
playAll (where playlistService.getPlaylistTracks(playlistId) returns the full
list) compute a fresh offline-playable set instead of using playableOfflineKeys:
for each track from getPlaylistTracks call getInternalPlayUri(track), collect
URIs, call Orpheus.getLruCachedUris(...) and check
downloadStatus[track.uniqueKey] === DownloadState.COMPLETED (and include local
source tracks) to build a Set of playable uniqueKeys, then filter the full
playlist by that Set before enqueueing/playing so unloaded but cached/downloaded
tracks are preserved.

In `@apps/mobile/src/features/player/components/PlayerControls.tsx`:
- Around line 173-186: Wrap the awaited Orpheus.play()/Orpheus.pause() calls
inside the onPress async handler in a try/catch to prevent unhandled promise
rejections; use the same debouncedIsPlaying / setDebouncedIsPlaying logic but
perform the await and analyticsService.logPlayerAction(...) inside the try
block, and in the catch block log the error (e.g., via your logger or analytics
error method) and ensure UI state remains consistent if the operation failed.
Target the onPress handler where debouncedIsPlaying, setDebouncedIsPlaying,
Orpheus.play, Orpheus.pause and analyticsService.logPlayerAction are used.
- Around line 190-204: The LottieView is remounted when debouncedBuffering
toggles, but the existing useEffect that sets the play/pause frame only depends
on debouncedIsPlaying so the Lottie may render the wrong frame; update the logic
so mounting respects current state—either add debouncedBuffering to the
dependency array of the effect that manipulates
playPauseLottieRef/debouncedIsPlaying (so it runs when buffering ends) or force
LottieView to remount with a key tied to debouncedBuffering and adjust the
isFirstMount logic accordingly so the playPauseLottieRef is initialized to the
correct frame on mount.

In `@apps/mobile/src/features/player/hooks/danmaku/useDanmakuLoader.ts`:
- Around line 86-89: The retry setTimeout in the catch block currently doesn't
store its timer ID, so pending timers can race across bvid/cid changes and
delete entries added for new content; update the catch logic in useDanmakuLoader
to save each timer ID (e.g., in a Map keyed by segIndex or a Set of timer IDs)
when calling setTimeout, ensure the cleanup branch of the surrounding useEffect
clears those timers (clearTimeout) and removes their entries, and also clear any
pending timers when loadedSegmentsRef.current.clear() is run so stale timers
cannot delete newly added segIndex entries.

In
`@packages/orpheus/android/src/main/java/expo/modules/orpheus/ExpoOrpheusModule.kt`:
- Around line 449-451: The batch call in addToEnd maps tracks with
track.toMediaItem(context) on Queues.MAIN causing repeated synchronous file I/O
via CoverDownloadManager.getCoverFile; move this work off the main thread or
eliminate sync I/O in TrackRecordExtension.kt. Fix by changing addToEnd to
perform the mapping on an IO dispatcher (e.g.,
launch/withContext(Dispatchers.IO) or equivalent) so toMediaItem(context) runs
off the main thread, or modify TrackRecordExtension.kt/
CoverDownloadManager.getCoverFile to return a non-blocking/cached result (async
API) and update toMediaItem to use that async result; reference symbols:
addToEnd, toMediaItem(context), CoverDownloadManager.getCoverFile,
TrackRecordExtension.kt, Queues.MAIN.

In
`@packages/orpheus/android/src/main/java/expo/modules/orpheus/util/TrackRecordExtension.kt`:
- Around line 17-19: toMediaItem(context) calls
expo.modules.orpheus.manager.CoverDownloadManager.getCoverFile(...) on the main
thread (invoked by addToEnd / Queues.MAIN and restoreQueue), causing sync
File.exists() I/O for each track; move cover-path resolution off the main thread
or cache results to avoid blocking: either introduce an in-memory Map cache
keyed by track id that toMediaItem consults before calling getCoverFile, or
perform cover lookups on a background IO dispatcher (build mediaItems in IO then
post to main thread for player updates). Also replace the fully-qualified
CoverDownloadManager reference with an import to keep code concise. Ensure
symbols to change include toMediaItem(context),
CoverDownloadManager.getCoverFile, addToEnd, and restoreQueue/Queues.MAIN.

---

Nitpick comments:
In `@apps/mobile/src/components/modals/PlayerQueueModal.tsx`:
- Line 120: 在 useEffect 里移除冗余的 refetch 调用:usePlayerQueue(isVisible) 已经通过 enabled
翻转在 isVisible 从 false->true 时触发拉取(staleTime: 0),所以不要再显式调用 refetch();保留对
didInitialScroll 的重置逻辑(当 isVisible 为 false 时重置),更新或删除 useEffect 中对 refetch
的那一行(涉及符号
usePlayerQueue、refetch、isVisible、didInitialScroll、useEffect)以简化逻辑且避免冗余调用。

In `@apps/mobile/src/features/player/components/PlayerControls.tsx`:
- Line 123: Replace the inappropriate wording in the inline comment inside the
PlayerControls component: locate the comment containing "tmd" and change it to a
neutral, professional phrase (e.g., "temporary workaround" or "hack") such as
"Note: this is a temporary workaround; colorFilters do not take effect." Ensure
the comment remains concise and explanatory and update only the comment text in
PlayerControls.tsx.

In `@apps/mobile/src/hooks/queries/bilibili/danmaku.ts`:
- Around line 16-22: The fetchQuery call for danmaku should explicitly set React
Query's retry behavior to avoid long exponential backoffs in the player; update
the queryClient.fetchQuery invocation (where you use danmakuQueryKeys.segment,
returnOrThrowAsync and bilibiliApi.getSegDanmaku) to include an options object
with retry: 0 (or a small integer) so failed danmaku fetches fall back quickly
instead of inheriting the default 3 retries.

In `@apps/mobile/src/hooks/queries/orpheus/index.ts`:
- Around line 57-60: In usePlayerQueue's queryFn, remove the unnecessary
async/await and intermediate variable: instead of awaiting Orpheus.getQueue()
into q and returning q, have queryFn return the Promise from Orpheus.getQueue()
directly (i.e., replace the async function body so it simply returns
Orpheus.getQueue()); this removes the redundant async wrapper while keeping
behavior identical. Reference: usePlayerQueue -> queryFn -> Orpheus.getQueue.
- Around line 44-52: useDownloadTasks currently omits an explicit gcTime: 0
option, relying on setQueryDefaults while other hooks set gcTime: 0 explicitly;
update the useDownloadTasks query options (in the useDownloadTasks function) to
include gcTime: 0 alongside staleTime: 0 so it matches the other hooks and
avoids confusion during maintenance.

In `@apps/mobile/src/lib/services/lyricService.ts`:
- Around line 224-233: Replace the use of ResultAsync.fromSafePromise with
ResultAsync.fromPromise when wrapping Network.getNetworkStateAsync() inside the
orElse chain in lyricService.ts so rejections are handled; ensure you map any
thrown error from Network.getNetworkStateAsync() into a retryable/handled
failure (e.g., return errAsync(new LyricNotFoundError(...)) or convert the
rejection to the same error path) and keep the subsequent logic that checks
networkState.isConnected and calls fetchFromNetwork() intact.

In `@apps/mobile/src/utils/lottie.ts`:
- Around line 14-16: 当前实现对 JSON.stringify(source).replace(/\[1,1,1,1\]/g, ...)
全局替换可能误中非颜色数组;在 apps/mobile/src/utils/lottie.ts 中的返回语句(返回值类型
AnimationObject)请改为更精确的替换:要么在替换中加入键上下文如匹配 /"k"\s*:\s*\[1,1,1,1\]/
或类似只匹配色彩字段的正则,确保只替换颜色属性;更稳健的做法是遍历 source 对象并在遇到预期的 color 属性(或特定图层/属性名)时替换
[1,1,1,1],同时在代码中加注释说明该约束以避免未来误用。
- Around line 10-13: The hex-to-normalized-RGB conversion lacks input validation
for hexColor; update the code that computes hex, r, g, b to first normalize and
validate hexColor (strip leading '#', accept 3- or 6‑char forms by expanding
3->6, ensure /^[0-9a-fA-F]{6}$/), and if validation fails use a safe fallback
(e.g., default "000000") or throw a clear error; after validation parseInt
results check for NaN before calling toFixed (or convert to numbers and then
toFixed) so JSON.parse will never receive "NaN" strings. Use the existing
hexColor variable and the hex/r/g/b computation block as the insertion point.

In `@apps/mobile/src/utils/time.ts`:
- Around line 37-44: 在 formatDurationToText(seconds: number) 中移除无效的
text.trim()(生成结果在两个分支均无前后空白),并可选性地在 hours > 0 且 minutes === 0 的情况下不输出 "0
分钟":即构建字符串时只在 hours > 0 时追加 `${hours} 小时`,再在 minutes > 0 时追加 `${minutes}
分钟`,确保返回值不再调用 trim()。

In
`@packages/orpheus/android/src/main/java/expo/modules/orpheus/ExpoOrpheusModule.kt`:
- Around line 764-772: The getLruCachedUris lambda currently swallows all
exceptions; modify it to catch the Exception from
expo.modules.orpheus.manager.CachedUriManager.isFullyCached and log the error
before returning an empty list so failures are visible; locate the
Function("getLruCachedUris") block and update its catch to call Android logging
(e.g., Log.e with a TAG) or the module logger and include the exception and
context (the uris or a short message) while preserving the emptyList<String>()
return behavior.

In
`@packages/orpheus/android/src/main/java/expo/modules/orpheus/manager/CachedUriManager.kt`:
- Around line 73-74: 在 CachedUriManager 类中移除空的 override 方法 onSpanTouched(cache:
Cache, oldSpan: CacheSpan, newSpan:
CacheSpan);该方法体为空且无需处理访问事件,直接删除整个方法覆盖(保留类声明不变)以消除 detekt 的 EmptyFunctionBlock
警告;若接口要求实现且不能删除,则替换为明确的 `@Suppress`("EmptyFunctionBlock") 注解并保留方法签名以抑制警告。

ℹ️ Review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7c19bac and f7109de.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (31)
  • apps/docs/docs/guides/playlist.md
  • apps/mobile/CHANGELOG.md
  • apps/mobile/package.json
  • apps/mobile/src/app/download.tsx
  • apps/mobile/src/app/playlist/local/[id].tsx
  • apps/mobile/src/assets/lottie/play-pause.json
  • apps/mobile/src/assets/lottie/skip-next.json
  • apps/mobile/src/assets/lottie/skip-prev.json
  • apps/mobile/src/components/modals/PlayerQueueModal.tsx
  • apps/mobile/src/components/modals/player/SleepTimerModal.tsx
  • apps/mobile/src/features/player/components/PlayerControls.tsx
  • apps/mobile/src/features/player/components/PlayerFunctionalMenu.tsx
  • apps/mobile/src/features/player/components/PlayerHeader.tsx
  • apps/mobile/src/features/player/hooks/danmaku/useDanmakuLoader.ts
  • apps/mobile/src/features/playlist/local/components/LocalPlaylistHeader.tsx
  • apps/mobile/src/features/playlist/local/components/LocalTrackList.tsx
  • apps/mobile/src/features/playlist/local/hooks/useLocalPlaylistPlayer.ts
  • apps/mobile/src/hooks/player/useBatchDownloadStatus.ts
  • apps/mobile/src/hooks/queries/bilibili/danmaku.ts
  • apps/mobile/src/hooks/queries/lyrics/index.ts
  • apps/mobile/src/hooks/queries/orpheus/index.ts
  • apps/mobile/src/lib/services/lyricService.ts
  • apps/mobile/src/lib/services/playlistService.ts
  • apps/mobile/src/utils/lottie.ts
  • apps/mobile/src/utils/time.ts
  • packages/orpheus/android/src/main/java/expo/modules/orpheus/ExpoOrpheusModule.kt
  • packages/orpheus/android/src/main/java/expo/modules/orpheus/manager/CachedUriManager.kt
  • packages/orpheus/android/src/main/java/expo/modules/orpheus/service/OrpheusMusicService.kt
  • packages/orpheus/android/src/main/java/expo/modules/orpheus/util/GeneralStorage.kt
  • packages/orpheus/android/src/main/java/expo/modules/orpheus/util/TrackRecordExtension.kt
  • packages/orpheus/src/ExpoOrpheusModule.ts
💤 Files with no reviewable changes (1)
  • apps/mobile/src/hooks/player/useBatchDownloadStatus.ts

Comment on lines +104 to +133
const playableOfflineKeys = useMemo(() => {
if (!allLoadedTracks.length) return new Set<string>()

const keys = new Set<string>()
const urisToCheck: { uniqueKey: string; uri: string }[] = []

for (const track of allLoadedTracks) {
if (track.source === 'local') {
keys.add(track.uniqueKey)
continue
}
const uri = getInternalPlayUri(track)
if (uri) {
urisToCheck.push({ uniqueKey: track.uniqueKey, uri })
}
}

const validUris = new Set(
Orpheus.getLruCachedUris(urisToCheck.map((u) => u.uri)),
)
for (const item of urisToCheck) {
if (
validUris.has(item.uri) ||
downloadStatus?.[item.uniqueKey] === DownloadState.COMPLETED
) {
keys.add(item.uniqueKey)
}
}
return keys
}, [allLoadedTracks, downloadStatus])
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

🏁 Script executed:

# First, let's find the file and understand its structure
find . -type f -name "[id].tsx" | grep -i playlist | head -20

Repository: bbplayer-app/BBPlayer

Length of output: 47


🏁 Script executed:

# Let's check if the file exists at the provided path
cat apps/mobile/src/app/playlist/local/\[id\].tsx | head -150

Repository: bbplayer-app/BBPlayer

Length of output: 5109


🏁 Script executed:

# Continue reading the file to find useLocalPlaylistPlayer usage and playAll callback
sed -n '130,250p' apps/mobile/src/app/playlist/local/\[id\].tsx

Repository: bbplayer-app/BBPlayer

Length of output: 2772


🏁 Script executed:

# Now let's find the useLocalPlaylistPlayer hook
find . -type f -name "*.ts" -o -name "*.tsx" | xargs grep -l "useLocalPlaylistPlayer" | head -5

Repository: bbplayer-app/BBPlayer

Length of output: 184


🏁 Script executed:

# Let's examine the useLocalPlaylistPlayer hook
cat apps/mobile/src/features/playlist/local/hooks/useLocalPlaylistPlayer.ts

Repository: bbplayer-app/BBPlayer

Length of output: 2856


🏁 Script executed:

# Let's check the getPlaylistTracks implementation
find . -type f -name "*.ts" -o -name "*.tsx" | xargs grep -l "getPlaylistTracks" | grep -i service

Repository: bbplayer-app/BBPlayer

Length of output: 114


🏁 Script executed:

# Let's examine the playlistService implementation
cat apps/mobile/src/lib/services/playlistService.ts | head -300

Repository: bbplayer-app/BBPlayer

Length of output: 7692


🏁 Script executed:

# Continue reading to find getPlaylistTracks method
sed -n '300,500p' apps/mobile/src/lib/services/playlistService.ts

Repository: bbplayer-app/BBPlayer

Length of output: 5479


🏁 Script executed:

# Continue searching for getPlaylistTracks
sed -n '500,700p' apps/mobile/src/lib/services/playlistService.ts

Repository: bbplayer-app/BBPlayer

Length of output: 5409


🏁 Script executed:

# Let's verify how usePlaylistContentsInfinite works and confirm pagination
find . -type f -name "*.ts" -o -name "*.tsx" | xargs grep -l "usePlaylistContentsInfinite"

Repository: bbplayer-app/BBPlayer

Length of output: 157


🏁 Script executed:

# Let's examine the usePlaylistContentsInfinite implementation
cat apps/mobile/src/hooks/queries/db/playlist.ts

Repository: bbplayer-app/BBPlayer

Length of output: 3860


🏁 Script executed:

# Let's check the getPlaylistTracksPaginated implementation
grep -n "getPlaylistTracksPaginated" apps/mobile/src/lib/services/playlistService.ts -A 80

Repository: bbplayer-app/BBPlayer

Length of output: 2595


离线模式下过滤逻辑存在分页数据不匹配问题,导致未加载页中的已缓存/已下载曲目被错误排除

playAll 通过 playlistService.getPlaylistTracks(playlistId) 获取播放列表中的全部曲目,但随后按 playableOfflineKeys 过滤,而 playableOfflineKeys 仅基于 allLoadedTracks(已加载页的数据)计算。

当用户在仅加载前几页时点击「播放全部」且处于离线模式,后续未加载页中已缓存/已下载的曲目不在 playableOfflineKeys 中,会被错误过滤掉。同时触发的 toast 提示"当前离线,仅添加可播放的已缓存歌曲到播放队列"误导用户。

建议方案:

  1. playAll 中独立重新计算离线可播放集合(基于获取到的全部曲目),而非依赖外部传入的 playableOfflineKeys
  2. 或在离线模式下先加载全部曲目元数据后再进行离线可用性计算。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/mobile/src/app/playlist/local/`[id].tsx around lines 104 - 133, playAll
currently filters the full playlist using playableOfflineKeys (which is derived
from allLoadedTracks/page-only data), causing cached/downloaded tracks in
unloaded pages to be excluded; inside playAll (where
playlistService.getPlaylistTracks(playlistId) returns the full list) compute a
fresh offline-playable set instead of using playableOfflineKeys: for each track
from getPlaylistTracks call getInternalPlayUri(track), collect URIs, call
Orpheus.getLruCachedUris(...) and check downloadStatus[track.uniqueKey] ===
DownloadState.COMPLETED (and include local source tracks) to build a Set of
playable uniqueKeys, then filter the full playlist by that Set before
enqueueing/playing so unloaded but cached/downloaded tracks are preserved.

Comment on lines 173 to 186
onPress={async () => {
onInteraction?.()
void Haptics.performHaptics(Haptics.AndroidHaptics.Context_Click)
const isPlaying = await Orpheus.getIsPlaying()
if (isPlaying) {

const nextIsPlaying = !debouncedIsPlaying
setDebouncedIsPlaying(nextIsPlaying)

if (debouncedIsPlaying) {
await Orpheus.pause()
void analyticsService.logPlayerAction('pause')
} else {
await Orpheus.play()
void analyticsService.logPlayerAction('play')
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Orpheus.play() / Orpheus.pause() 调用缺少错误处理。

onPress 使用了 asyncawait 播放/暂停操作,但没有 try/catch。如果操作失败(例如播放器实例已销毁),异常会变成 unhandled promise rejection。考虑到 CHANGELOG 中提到了"播放器实例被销毁后自动重建"的场景,这里的错误处理尤为重要。

💡 建议添加 try/catch
 			onPress={async () => {
 				onInteraction?.()
 				void Haptics.performHaptics(Haptics.AndroidHaptics.Context_Click)

 				const nextIsPlaying = !debouncedIsPlaying
 				setDebouncedIsPlaying(nextIsPlaying)

-				if (debouncedIsPlaying) {
-					await Orpheus.pause()
-					void analyticsService.logPlayerAction('pause')
-				} else {
-					await Orpheus.play()
-					void analyticsService.logPlayerAction('play')
+				try {
+					if (debouncedIsPlaying) {
+						await Orpheus.pause()
+						void analyticsService.logPlayerAction('pause')
+					} else {
+						await Orpheus.play()
+						void analyticsService.logPlayerAction('play')
+					}
+				} catch (error) {
+					// 播放/暂停失败时回滚乐观更新
+					setDebouncedIsPlaying(debouncedIsPlaying)
 				}
 			}}
📝 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
onPress={async () => {
onInteraction?.()
void Haptics.performHaptics(Haptics.AndroidHaptics.Context_Click)
const isPlaying = await Orpheus.getIsPlaying()
if (isPlaying) {
const nextIsPlaying = !debouncedIsPlaying
setDebouncedIsPlaying(nextIsPlaying)
if (debouncedIsPlaying) {
await Orpheus.pause()
void analyticsService.logPlayerAction('pause')
} else {
await Orpheus.play()
void analyticsService.logPlayerAction('play')
}
onPress={async () => {
onInteraction?.()
void Haptics.performHaptics(Haptics.AndroidHaptics.Context_Click)
const nextIsPlaying = !debouncedIsPlaying
setDebouncedIsPlaying(nextIsPlaying)
try {
if (debouncedIsPlaying) {
await Orpheus.pause()
void analyticsService.logPlayerAction('pause')
} else {
await Orpheus.play()
void analyticsService.logPlayerAction('play')
}
} catch (error) {
// 播放/暂停失败时回滚乐观更新
setDebouncedIsPlaying(debouncedIsPlaying)
}
}}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/mobile/src/features/player/components/PlayerControls.tsx` around lines
173 - 186, Wrap the awaited Orpheus.play()/Orpheus.pause() calls inside the
onPress async handler in a try/catch to prevent unhandled promise rejections;
use the same debouncedIsPlaying / setDebouncedIsPlaying logic but perform the
await and analyticsService.logPlayerAction(...) inside the try block, and in the
catch block log the error (e.g., via your logger or analytics error method) and
ensure UI state remains consistent if the operation failed. Target the onPress
handler where debouncedIsPlaying, setDebouncedIsPlaying, Orpheus.play,
Orpheus.pause and analyticsService.logPlayerAction are used.

Comment on lines +190 to +204
{debouncedBuffering ? (
<ActivityIndicator
size={playButtonSize * 0.4}
color={colors.primary}
/>
) : (
<LottieView
ref={playPauseLottieRef}
source={tintedPlayPause}
style={{ width: '100%', height: '100%' }}
autoPlay={false}
speed={2}
loop={false}
/>
)}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

缓冲结束后 LottieView 重新挂载,播放/暂停按钮可能显示错误帧。

debouncedBufferingtrue 变为 false 时,ActivityIndicator 被卸载,LottieView 重新挂载。但此时 debouncedIsPlaying 未发生变化,Line 101-117 的 useEffect 不会重新执行,导致 LottieView 以默认帧(帧 0,即暂停图标)渲染,而不是反映当前的实际播放状态。

建议在 LottieView 挂载时也根据当前状态初始化正确的帧,例如将缓冲状态恢复也纳入 useEffect 的依赖中,或在 LottieView 重新挂载时使用 key 触发重新初始化。

💡 一种可能的修复思路

可以在 useEffect 中加入 debouncedBuffering 作为依赖,在缓冲结束时强制设置正确的帧:

 	useEffect(() => {
-		if (isFirstMount.current) {
-			isFirstMount.current = false
-			if (debouncedIsPlaying) {
-				playPauseLottieRef.current?.play(0, 0)
-			} else {
-				playPauseLottieRef.current?.play(8, 8)
-			}
-			return
-		}
-
-		if (debouncedIsPlaying) {
-			playPauseLottieRef.current?.play(8, 0)
+		if (isFirstMount.current || !debouncedBuffering) {
+			// 首次挂载或缓冲结束后,跳转到正确的静态帧
+			if (isFirstMount.current) {
+				isFirstMount.current = false
+			}
+			if (debouncedIsPlaying) {
+				playPauseLottieRef.current?.play(0, 0)
+			} else {
+				playPauseLottieRef.current?.play(8, 8)
+			}
 		} else {
-			playPauseLottieRef.current?.play(0, 8)
+			if (debouncedIsPlaying) {
+				playPauseLottieRef.current?.play(8, 0)
+			} else {
+				playPauseLottieRef.current?.play(0, 8)
+			}
 		}
-	}, [debouncedIsPlaying])
+	}, [debouncedIsPlaying, debouncedBuffering])

或者更简洁地,使用 key={debouncedBuffering} 强制 LottieView 在缓冲状态变化时重新挂载并触发 isFirstMount 逻辑(但需要相应调整 isFirstMount 的管理方式)。

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/mobile/src/features/player/components/PlayerControls.tsx` around lines
190 - 204, The LottieView is remounted when debouncedBuffering toggles, but the
existing useEffect that sets the play/pause frame only depends on
debouncedIsPlaying so the Lottie may render the wrong frame; update the logic so
mounting respects current state—either add debouncedBuffering to the dependency
array of the effect that manipulates playPauseLottieRef/debouncedIsPlaying (so
it runs when buffering ends) or force LottieView to remount with a key tied to
debouncedBuffering and adjust the isFirstMount logic accordingly so the
playPauseLottieRef is initialized to the correct frame on mount.

Comment on lines +86 to +89
loadedSegmentsRef.current.add(segIndex)
setTimeout(() => {
loadedSegmentsRef.current.delete(segIndex)
}, delay)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

未取消的重试计时器可能与 bvid/cid 切换竞争

setTimeout 的返回 ID 未被保存,重置 Effect(第 138-143 行)也不会取消挂起的计时器。当 bvid/cid 切换后:

  1. loadedSegmentsRef.current.clear() 清空集合
  2. 新内容立即开始加载 segIndex(例如 1),并将其加入 loadedSegmentsRef
  3. 旧内容的重试计时器触发,执行 loadedSegmentsRef.current.delete(segIndex)——这会把新内容刚加入的项误删掉,触发一次多余的重新抓取

建议在 useEffect 中维护并清理待定的计时器 ID。

🛡️ 建议修复:追踪并清理重试计时器
-	const retryCountRef = useRef<Record<number, number>>({})
+	const retryCountRef = useRef<Record<number, number>>({})
+	const retryTimersRef = useRef<Record<number, ReturnType<typeof setTimeout>>>({})

catch 块中:

-			setTimeout(() => {
+			const timerId = setTimeout(() => {
 				loadedSegmentsRef.current.delete(segIndex)
+				delete retryTimersRef.current[segIndex]
 			}, delay)
+			retryTimersRef.current[segIndex] = timerId

在重置 Effect 中:

 	useEffect(() => {
 		rawDataSV.set([])
 		loadedSegmentsRef.current.clear()
 		isLoadingRef.current = false
 		retryCountRef.current = {}
+		Object.values(retryTimersRef.current).forEach(clearTimeout)
+		retryTimersRef.current = {}
 	}, [bvid, cid, rawDataSV])
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/mobile/src/features/player/hooks/danmaku/useDanmakuLoader.ts` around
lines 86 - 89, The retry setTimeout in the catch block currently doesn't store
its timer ID, so pending timers can race across bvid/cid changes and delete
entries added for new content; update the catch logic in useDanmakuLoader to
save each timer ID (e.g., in a Map keyed by segIndex or a Set of timer IDs) when
calling setTimeout, ensure the cleanup branch of the surrounding useEffect
clears those timers (clearTimeout) and removes their entries, and also clear any
pending timers when loadedSegmentsRef.current.clear() is run so stale timers
cannot delete newly added segIndex entries.

Comment on lines +449 to +451
val context = appContext.reactContext
val mediaItems = tracks.map { track ->
track.toMediaItem()
track.toMediaItem(context)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

addToEnd 在主线程批量调用 toMediaItem(context) 可能触发多次文件 I/O

toMediaItem(context) 内部会调用 CoverDownloadManager.getCoverFileFile.exists() 系统调用)。当 tracks 列表较大时(如一次性导入整张专辑/播放列表),在 Queues.MAIN 上遍历所有 track 会累积同步文件 I/O,可能导致明显卡顿。详见 TrackRecordExtension.kt 中对应问题,根本修复应在那里进行。

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@packages/orpheus/android/src/main/java/expo/modules/orpheus/ExpoOrpheusModule.kt`
around lines 449 - 451, The batch call in addToEnd maps tracks with
track.toMediaItem(context) on Queues.MAIN causing repeated synchronous file I/O
via CoverDownloadManager.getCoverFile; move this work off the main thread or
eliminate sync I/O in TrackRecordExtension.kt. Fix by changing addToEnd to
perform the mapping on an IO dispatcher (e.g.,
launch/withContext(Dispatchers.IO) or equivalent) so toMediaItem(context) runs
off the main thread, or modify TrackRecordExtension.kt/
CoverDownloadManager.getCoverFile to return a non-blocking/cached result (async
API) and update toMediaItem to use that async result; reference symbols:
addToEnd, toMediaItem(context), CoverDownloadManager.getCoverFile,
TrackRecordExtension.kt, Queues.MAIN.

Comment on lines +17 to +19
val downloadedCoverUri = context?.let {
expo.modules.orpheus.manager.CoverDownloadManager.getCoverFile(it, this.id)?.absolutePath?.let { path -> "file://$path" }
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

getCoverFile 在主线程执行文件 I/O,大批量调用时可能造成卡顿

toMediaItem(context)addToEndQueues.MAIN)和 restoreQueue(Service onCreate,主线程)调用。当传入的 tracks 列表较大时,每个 track 都会触发 getCoverFile 中的 File.exists() 系统调用,累积延迟在低端设备上可能导致明显丢帧。

建议将该查询结果缓存(内存 Map),或将 cover 路径的解析移至 IO 线程(例如在 addToEnd 中先在后台线程构建 mediaItems 再回到主线程操作 player)。


建议使用 import 替代全限定类名(次要)

♻️ 建议改动
 import expo.modules.orpheus.model.TrackRecord
+import expo.modules.orpheus.manager.CoverDownloadManager
 import kotlinx.serialization.encodeToString
-        expo.modules.orpheus.manager.CoverDownloadManager.getCoverFile(it, this.id)?.absolutePath?.let { path -> "file://$path" }
+        CoverDownloadManager.getCoverFile(it, this.id)?.absolutePath?.let { path -> "file://$path" }
📝 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
val downloadedCoverUri = context?.let {
expo.modules.orpheus.manager.CoverDownloadManager.getCoverFile(it, this.id)?.absolutePath?.let { path -> "file://$path" }
}
val downloadedCoverUri = context?.let {
CoverDownloadManager.getCoverFile(it, this.id)?.absolutePath?.let { path -> "file://$path" }
}
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@packages/orpheus/android/src/main/java/expo/modules/orpheus/util/TrackRecordExtension.kt`
around lines 17 - 19, toMediaItem(context) calls
expo.modules.orpheus.manager.CoverDownloadManager.getCoverFile(...) on the main
thread (invoked by addToEnd / Queues.MAIN and restoreQueue), causing sync
File.exists() I/O for each track; move cover-path resolution off the main thread
or cache results to avoid blocking: either introduce an in-memory Map cache
keyed by track id that toMediaItem consults before calling getCoverFile, or
perform cover lookups on a background IO dispatcher (build mediaItems in IO then
post to main thread for player updates). Also replace the fully-qualified
CoverDownloadManager reference with an import to keep code concise. Ensure
symbols to change include toMediaItem(context),
CoverDownloadManager.getCoverFile, addToEnd, and restoreQueue/Queues.MAIN.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

♻️ Duplicate comments (1)
apps/mobile/src/app/playlist/local/[id].tsx (1)

120-149: 离线模式下 playableOfflineKeys 仍仅基于已加载分页数据计算,未加载页中已缓存曲目在「播放全部」时会被错误排除。

此问题与上次审查中指出的一致——playableOfflineKeysallLoadedTracks(仅已加载分页)构建,而 playAll 会获取全部曲目后用此集合过滤,导致未加载分页中已缓存/已下载的曲目被遗漏。

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/mobile/src/app/playlist/local/`[id].tsx around lines 120 - 149,
playableOfflineKeys is being computed only from allLoadedTracks (paged subset)
so cached/downloaded tracks in unloaded pages are excluded when playAll filters
by this set; update the logic to compute playableOfflineKeys from the full
playlist used for playback rather than allLoadedTracks — either move the
computation out of the component into a function that accepts the complete
tracks array (the same list playAll uses) or change the dependency from
allLoadedTracks to the full tracks list, running the same steps (check
track.source, call getInternalPlayUri, query Orpheus.getLruCachedUris, and test
downloadStatus[uniqueKey] === DownloadState.COMPLETED) against every track so
unloaded-but-cached or downloaded items are included.
🧹 Nitpick comments (2)
apps/mobile/src/app/playlist/local/[id].tsx (2)

58-63: SELECT_MODE_ITEM_HEIGHT = 69 硬编码存在布局漂移风险。

拖拽位置计算完全依赖此常量与实际渲染高度精确匹配。如果 TrackListItem 的 padding、字体大小或 Divider 高度发生变化,插入指示线会逐渐偏移。建议在注释中标注此值的来源或关联的样式,方便日后同步维护。

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/mobile/src/app/playlist/local/`[id].tsx around lines 58 - 63, The
constant SELECT_MODE_ITEM_HEIGHT is hardcoded and used for drag position math
(risking layout drift if TrackListItem/Divider styles change); update the code
to either compute the item height dynamically from the rendered TrackListItem
element or, at minimum, add a clear comment tying SELECT_MODE_ITEM_HEIGHT to the
specific CSS rules (padding, font-size, Divider height) that determine its value
so future style changes are synchronized—locate SELECT_MODE_ITEM_HEIGHT and the
drag/auto-scroll logic that references it (and the TrackListItem/Divider
components) and replace the magic number with a runtime measurement or document
its exact source in the code comment.

330-341: 自动滚动中 scrollOffsetRef 的乐观更新在列表底部边界可能短暂失准。

scrollOffsetRef.current = next 在实际滚动完成前更新,向下自动滚动超过内容底部时,FlashList 会钳制实际偏移,但 ref 值会偏大,直到下一次 onScroll 事件修正。这会导致 updateDragPositionhoverRel 短暂偏大、插入线位置跳动。

可以考虑在 onScroll 回调中更新 ref 后,若正在拖拽则重新调用一次 updateDragPosition,或在自动滚动中从 FlashList 的实际 offset 读取值。

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/mobile/src/app/playlist/local/`[id].tsx around lines 330 - 341, The
optimistic update of scrollOffsetRef in startAutoScroll can drift when FlashList
clamps the actual offset; fix by making the onScroll handler authoritative: in
your onScroll callback (where you currently read scroll events), set
scrollOffsetRef.current = eventOffset (actual native offset) and if a drag is in
progress call updateDragPosition() so the drag hover updates immediately; keep
startAutoScroll (function startAutoScroll) to request scrolls but do not rely on
its optimistic assignment as the sole source of truth—either remove the inline
scrollOffsetRef.current = next or keep it as a temporary guess but ensure
onScroll overwrites it and triggers updateDragPosition when dragging. Ensure you
reference autoScrollRef, scrollOffsetRef, listRef, startAutoScroll,
stopAutoScroll, onScroll, and updateDragPosition in your changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@apps/mobile/CHANGELOG.md`:
- Line 15: The changelog entry "- 优化无网状态下的本地播放列表显示逻辑,高亮已下载和自动缓存的歌曲。" ends with a
Chinese full stop "。" which is inconsistent with other entries; remove the
trailing "。" from this entry so it reads "- 优化无网状态下的本地播放列表显示逻辑,高亮已下载和自动缓存的歌曲" to
match the file's punctuation style.

In `@apps/mobile/src/features/playlist/local/components/LocalTrackList.tsx`:
- Line 18: The import of MD3Colors uses an internal path which is unstable;
update the import in LocalTrackList.tsx to use the public API by replacing the
internal import of MD3Colors with a type import from 'react-native-paper' (i.e.,
import type { MD3Colors } from 'react-native-paper') so the component/type
references (MD3Colors) resolve via the supported public package surface.

---

Duplicate comments:
In `@apps/mobile/src/app/playlist/local/`[id].tsx:
- Around line 120-149: playableOfflineKeys is being computed only from
allLoadedTracks (paged subset) so cached/downloaded tracks in unloaded pages are
excluded when playAll filters by this set; update the logic to compute
playableOfflineKeys from the full playlist used for playback rather than
allLoadedTracks — either move the computation out of the component into a
function that accepts the complete tracks array (the same list playAll uses) or
change the dependency from allLoadedTracks to the full tracks list, running the
same steps (check track.source, call getInternalPlayUri, query
Orpheus.getLruCachedUris, and test downloadStatus[uniqueKey] ===
DownloadState.COMPLETED) against every track so unloaded-but-cached or
downloaded items are included.

---

Nitpick comments:
In `@apps/mobile/src/app/playlist/local/`[id].tsx:
- Around line 58-63: The constant SELECT_MODE_ITEM_HEIGHT is hardcoded and used
for drag position math (risking layout drift if TrackListItem/Divider styles
change); update the code to either compute the item height dynamically from the
rendered TrackListItem element or, at minimum, add a clear comment tying
SELECT_MODE_ITEM_HEIGHT to the specific CSS rules (padding, font-size, Divider
height) that determine its value so future style changes are synchronized—locate
SELECT_MODE_ITEM_HEIGHT and the drag/auto-scroll logic that references it (and
the TrackListItem/Divider components) and replace the magic number with a
runtime measurement or document its exact source in the code comment.
- Around line 330-341: The optimistic update of scrollOffsetRef in
startAutoScroll can drift when FlashList clamps the actual offset; fix by making
the onScroll handler authoritative: in your onScroll callback (where you
currently read scroll events), set scrollOffsetRef.current = eventOffset (actual
native offset) and if a drag is in progress call updateDragPosition() so the
drag hover updates immediately; keep startAutoScroll (function startAutoScroll)
to request scrolls but do not rely on its optimistic assignment as the sole
source of truth—either remove the inline scrollOffsetRef.current = next or keep
it as a temporary guess but ensure onScroll overwrites it and triggers
updateDragPosition when dragging. Ensure you reference autoScrollRef,
scrollOffsetRef, listRef, startAutoScroll, stopAutoScroll, onScroll, and
updateDragPosition in your changes.

ℹ️ Review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f7109de and 0495dcf.

📒 Files selected for processing (6)
  • apps/mobile/CHANGELOG.md
  • apps/mobile/src/app/playlist/local/[id].tsx
  • apps/mobile/src/features/playlist/local/components/LocalPlaylistItem.tsx
  • apps/mobile/src/features/playlist/local/components/LocalTrackList.tsx
  • apps/mobile/src/hooks/mutations/db/playlist.ts
  • apps/mobile/src/lib/services/playlistService.ts

Comment thread apps/mobile/CHANGELOG.md Outdated
TouchableRipple,
useTheme,
} from 'react-native-paper'
import { MD3Colors } from 'react-native-paper/src/types'
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🌐 Web query:

react-native-paper MD3Colors public API v5 type export

💡 Result:

In react-native-paper v5, MD3Colors is part of the public API and is exported from the package root in two different ways:

  • As a TypeScript type (the theme color shape):

    import type { MD3Colors } from 'react-native-paper';

    types.d.ts exports MD3Colors as a type used by MD3Theme.colors. [1]

  • As a runtime constant (the MD3 color tokens like error50, primary90, etc.):

    import { MD3Colors } from 'react-native-paper';

    The docs show MD3Colors.error50, etc. [2]

If you need both in the same file, alias them:

import { MD3Colors as MD3ColorTokens } from 'react-native-paper';
import type { MD3Colors as MD3ColorsType } from 'react-native-paper';

[1][2]


🏁 Script executed:

cat -n apps/mobile/src/features/playlist/local/components/LocalTrackList.tsx | head -30

Repository: bbplayer-app/BBPlayer

Length of output: 1307


🏁 Script executed:

rg "MD3Colors" apps/mobile/src/features/playlist/local/components/LocalTrackList.tsx -A 2 -B 2

Repository: bbplayer-app/BBPlayer

Length of output: 321


MD3Colors 改为从公开 API 导入。

当前从内部路径 react-native-paper/src/types 导入不稳定,升级包版本时可能中断。MD3Colors 在 react-native-paper v5+ 中已从公开 API 导出,改为:

import type { MD3Colors } from 'react-native-paper'
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/mobile/src/features/playlist/local/components/LocalTrackList.tsx` at
line 18, The import of MD3Colors uses an internal path which is unstable; update
the import in LocalTrackList.tsx to use the public API by replacing the internal
import of MD3Colors with a type import from 'react-native-paper' (i.e., import
type { MD3Colors } from 'react-native-paper') so the component/type references
(MD3Colors) resolve via the supported public package surface.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (1)
apps/mobile/src/features/playlist/local/hooks/useTrackSelection.ts (1)

21-26: 建议在不传 id 时也显式重置 selected,以避免潜在的脏状态

enterSelectMode() 不携带 id 调用时,setSelected 完全不执行,selected 保留当前值。正常流程(exitSelectModeenterSelectMode)下不会有问题,因为 exitSelectMode 已经清空了集合。但由于 setSelected 被直接暴露在返回值中,消费者可以在任意时刻修改 selected,若此时再以无参方式调用 enterSelectMode(),用户将在毫无感知的情况下进入带有残留选中项的选择模式。

显式重置可使函数语义自洽,无需依赖调用顺序的隐式保证:

♻️ 建议的修改
 const enterSelectMode = useCallback((id?: number) => {
 	setSelectMode(true)
-	if (id !== undefined) {
-		setSelected(new Set([id]))
-	}
+	setSelected(id !== undefined ? new Set([id]) : new Set())
 }, [])
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/mobile/src/features/playlist/local/hooks/useTrackSelection.ts` around
lines 21 - 26, enterSelectMode currently only clears selected when given an id,
leaving potential stale selections if callers invoke enterSelectMode() with no
args while selected was modified externally; update enterSelectMode to always
reset selected (call setSelected(new Set()) when id is undefined) and then, if
id is provided, setSelected(new Set([id])); this keeps enterSelectMode's
semantics self-contained and prevents relying on exitSelectMode or external
callers mutating selected.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@apps/mobile/src/app/playlist/local/`[id].tsx:
- Around line 320-342: The auto-scroll interval started in startAutoScroll
(stored in autoScrollRef) isn't guaranteed to be cleared on component unmount or
route change; add a useEffect with a cleanup that calls stopAutoScroll to clear
any remaining interval when the component unmounts (and include stopAutoScroll
in the effect deps), ensuring autoScrollRef is set to null and clearInterval is
invoked if needed; update the component to import/use that effect so lingering
timers from startAutoScroll/autoScrollRef are always cleaned up.
- Around line 420-436: The drag-reorder logic uses clamped index into
allLoadedOrderValues while clamped is derived from finalPlaylistData (which may
be filtered), causing wrong order values to be written; change handleDragEnd so
it resolves the target order by trackId instead of by visual index: build a
mapping (e.g. orderByTrackId) from allLoadedOrderValues keyed by trackId and
look up targetOrder = orderByTrackId[targetTrackId] (use
currentDragging.trackId) before calling reorderTrack({ playlistId: Number(id),
trackId, toOrder: targetOrder }), and add orderByTrackId to the handleDragEnd
dependency array; alternatively disable drag while search/filter is active.

---

Nitpick comments:
In `@apps/mobile/src/features/playlist/local/hooks/useTrackSelection.ts`:
- Around line 21-26: enterSelectMode currently only clears selected when given
an id, leaving potential stale selections if callers invoke enterSelectMode()
with no args while selected was modified externally; update enterSelectMode to
always reset selected (call setSelected(new Set()) when id is undefined) and
then, if id is provided, setSelected(new Set([id])); this keeps
enterSelectMode's semantics self-contained and prevents relying on
exitSelectMode or external callers mutating selected.

ℹ️ Review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0495dcf and 147d0ed.

📒 Files selected for processing (2)
  • apps/mobile/src/app/playlist/local/[id].tsx
  • apps/mobile/src/features/playlist/local/hooks/useTrackSelection.ts

Comment thread apps/mobile/src/app/playlist/local/[id].tsx
Comment thread apps/mobile/src/app/playlist/local/[id].tsx
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

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

⚠️ Outside diff range comments (1)
apps/mobile/src/app/playlist/local/[id].tsx (1)

157-173: ⚠️ Potential issue | 🟡 Minor

batchAddTracksModalPayloads:提前 return [] 会丢弃所有已收集的有效 payload,且循环内 flatMap 导致 O(n×m) 复杂度。

两个问题:

  1. 遍历 selected 时若任意 trackId 在已加载页中找不到(如数据失效/重新拉取期间),立即 return [] 会将之前已收集的所有 payload 全部丢弃,用户将看到空的"添加到播放列表"弹窗而无任何提示。
  2. 每次迭代都调用 flatMap + find,时间复杂度为 O(|selected| × |allLoadedTracks|),应改为先构建 Map 再查找。
🛠️ 建议修复
-const batchAddTracksModalPayloads = (() => {
-    const payloads = []
-    for (const trackId of selected) {
-        const track = (playlistData?.pages as Array<{ tracks: Track[] }>)
-            ?.flatMap((page) => page.tracks)
-            .find((t) => t.id === trackId)
-        if (!track) return []
-        payloads.push({
-            track: { ...track, artistId: track.artist?.id },
-            artist: track.artist!,
-        })
-    }
-    return payloads
-})()
+const trackById = useMemo(() => {
+    const map = new Map<number, Track>()
+    for (const track of allLoadedTracks) map.set(track.id, track)
+    return map
+}, [allLoadedTracks])
+
+const batchAddTracksModalPayloads = useMemo(() => {
+    const payloads = []
+    for (const trackId of selected) {
+        const track = trackById.get(trackId)
+        if (!track) continue  // 跳过找不到的曲目,不丢弃其余已收集项
+        payloads.push({
+            track: { ...track, artistId: track.artist?.id },
+            artist: track.artist!,
+        })
+    }
+    return payloads
+}, [selected, trackById])
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/mobile/src/app/playlist/local/`[id].tsx around lines 157 - 173,
batchAddTracksModalPayloads currently returns [] early when any selected trackId
is missing and rebuilds the flat list on every iteration; instead, build a
single lookup Map from playlistData?.pages (flatten pages->tracks once) keyed by
track.id, then iterate selected and for each trackId do a Map.get—if missing,
skip that id (do not return []) so previously collected payloads are preserved;
when pushing payloads keep the same shape ({ track: { ...track, artistId:
track.artist?.id }, artist: track.artist! }) and ensure you handle undefined
artist safely.
♻️ Duplicate comments (4)
apps/mobile/src/app/playlist/local/[id].tsx (3)

126-155: playableOfflineKeys 仅覆盖已加载分页,离线过滤存在遗漏。

此问题在之前的评审中已标记:playableOfflineKeys 基于 allLoadedTracks(仅已加载的分页)计算,而 playAll 获取全部曲目后以此过滤,导致未加载分页中已缓存/已下载的曲目被错误排除。

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/mobile/src/app/playlist/local/`[id].tsx around lines 126 - 155,
playableOfflineKeys is currently computed from allLoadedTracks (only loaded
pages) causing unloaded-but-cached/downloaded tracks to be excluded; change the
useMemo to iterate the complete track list used by playAll (the full
playlist/tracks variable that contains all items) instead of allLoadedTracks,
still calling getInternalPlayUri for non-local tracks, querying
Orpheus.getLruCachedUris for URIs and checking downloadStatus[...] ===
DownloadState.COMPLETED, and update the dependency array to include that full
tracks variable and downloadStatus so offline-playable keys cover every track in
the playlist.

329-347: 组件卸载时自动滚动定时器未清理。

此问题在之前的评审中已标记:autoScrollRefsetInterval 未在组件卸载时通过 useEffect 清理,若用户在拖拽过程中返回上一页,定时器将残留。

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/mobile/src/app/playlist/local/`[id].tsx around lines 329 - 347, The
auto-scroll interval stored in autoScrollRef isn't cleared on component unmount;
add a useEffect that runs once and returns a cleanup which calls stopAutoScroll
to clear any existing interval. Reference the existing stopAutoScroll and
autoScrollRef/startAutoScroll functions: import or use stopAutoScroll in the
effect's cleanup and include it in the effect dependencies so the interval is
always cleared when the component unmounts or when stopAutoScroll changes.

415-469: 搜索/过滤状态下拖拽重排会写入错误的 sort_key

此问题在之前的评审中已标记:allLoadedSortKeys[clamped]finalPlaylistData(可能是过滤后的子集)的视觉索引取值,当搜索激活时与 allLoadedSortKeys(平行于 allLoadedTracks)的索引不对齐,会将错误的 sort_key 写入数据库。

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/mobile/src/app/playlist/local/`[id].tsx around lines 415 - 469, The drag
handler uses finalPlaylistData visual indexes to read allLoadedSortKeys
directly, which is wrong when finalPlaylistData is a filtered subset; update
handleDragEnd to map the visual neighbor(s) to their positions in the full list
before picking sort keys: determine the neighbor track IDs from
finalPlaylistData at the computed clamped/neighbor visual positions, find each
neighbor's index in allLoadedTracks (e.g. via findIndex by id), then read
prevSortKey/nextSortKey from allLoadedSortKeys at those full-list indexes and
pass them to reorderTrack (use draggingRef.current.trackId,
insertAfterIndexRef.current, finalPlaylistData, allLoadedTracks,
allLoadedSortKeys, and reorderTrack to implement this mapping).
apps/mobile/CHANGELOG.md (1)

15-15: 条目末尾标点符号不一致(重复)。

该条目末尾有中文句号"。",而文件中其他所有条目均不以标点结尾,已在之前的评审中标记。

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/mobile/CHANGELOG.md` at line 15, The changelog entry
"优化无网状态下的本地播放列表显示逻辑,高亮已下载和自动缓存的歌曲。" ends with a Chinese period unlike other
entries; remove the trailing "。" so it matches the file's style (no terminal
punctuation) and keep the rest of the text unchanged.
🧹 Nitpick comments (4)
apps/mobile/src/app/playlist/local/[id].tsx (1)

101-115: 重复类型断言可提取为局部变量。

(playlistData?.pages as Array<{ tracks: Track[]; sortKeys: string[] }>)allLoadedTracksallLoadedSortKeys 以及第 160 行的 IIFE 中重复出现,可提取一次。

♻️ 建议简化
+const loadedPages = playlistData?.pages as
+    | Array<{ tracks: Track[]; sortKeys: string[] }>
+    | undefined

 const allLoadedTracks = useMemo(
-    () => (playlistData?.pages as Array<{ tracks: Track[]; sortKeys: string[] }>)
-        ?.flatMap((page) => page.tracks) ?? [],
+    () => loadedPages?.flatMap((page) => page.tracks) ?? [],
     [playlistData],
 )
 const allLoadedSortKeys = useMemo(
-    () => (playlistData?.pages as Array<{ tracks: Track[]; sortKeys: string[] }>)
-        ?.flatMap((page) => page.sortKeys) ?? [],
+    () => loadedPages?.flatMap((page) => page.sortKeys) ?? [],
     [playlistData],
 )
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/mobile/src/app/playlist/local/`[id].tsx around lines 101 - 115, Extract
the repeated type assertion into a local variable (e.g., const pages =
playlistData?.pages as Array<{ tracks: Track[]; sortKeys: string[] }> ) and then
replace the three occurrences inside the allLoadedTracks useMemo,
allLoadedSortKeys useMemo and the IIFE at the later block with that variable;
this keeps the type assertion in one place while the existing symbols
(playlistData, allLoadedTracks, allLoadedSortKeys, and the IIFE that reads
pages) continue to work unchanged.
apps/mobile/src/hooks/app/useFastMigrations.ts (1)

65-78: 迁移在大型播放列表下可能造成短暂的 UI 阻塞。

migrateSortKeys 是同步函数,在主线程上对每一行逐条执行 UPDATE。如果用户有大量 sort_key = '' 的行(例如数千首歌曲),启动时可能会有明显的卡顿。由于该迁移仅执行一次(MMKV 守卫),影响有限,但可以考虑批量 UPDATE 或使用 CASE WHEN 来减少逐行写入的开销。

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/mobile/src/hooks/app/useFastMigrations.ts` around lines 65 - 78, The
current migrateSortKeys logic iterates and calls expoDb.runSync per row (inside
expoDb.withTransactionSync), causing UI jank for large playlists; change it to
produce and execute a single batched UPDATE per playlist: for each playlistId in
the grouped loop use generateKeyBetween to compute all sort keys into arrays,
then build one SQL update using a CASE WHEN mapping track_id -> sort_key with a
WHERE playlist_id = ? AND track_id IN (...) (or a single parameterized multi-row
UPDATE), and call expoDb.runSync once per playlist (still inside
expoDb.withTransactionSync) instead of calling expoDb.runSync for every
track_id. Ensure you reference generateKeyBetween, expoDb.withTransactionSync,
and expoDb.runSync when making the change.
apps/mobile/src/lib/db/schema.ts (1)

132-137: playlist_tracks_playlist_idx 现已被新复合索引覆盖,可考虑移除。

新增的 playlist_tracks_sort_key_idx(playlistId, sortKey) 的复合索引,它已经覆盖了 playlist_tracks_playlist_idx(仅 playlistId)的所有查询场景。保留两者会在写入时增加不必要的索引维护开销。

♻️ 移除冗余索引
 	(table) => [
 		primaryKey({ columns: [table.playlistId, table.trackId] }),
-		index('playlist_tracks_playlist_idx').on(table.playlistId),
 		index('playlist_tracks_track_idx').on(table.trackId),
 		index('playlist_tracks_sort_key_idx').on(table.playlistId, table.sortKey),
 	],
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/mobile/src/lib/db/schema.ts` around lines 132 - 137, 移除冗余的单列索引
playlist_tracks_playlist_idx:在定义表索引的数组中删除
index('playlist_tracks_playlist_idx').on(table.playlistId) 因为已存在的复合索引
index('playlist_tracks_sort_key_idx').on(table.playlistId, table.sortKey)
覆盖了所有基于 table.playlistId 的查询,保留 primaryKey(...) 和 playlist_tracks_sort_key_idx
即可减少写入时的索引维护开销。
apps/mobile/src/lib/services/playlistService.ts (1)

396-453: 添加前置校验以提供更清晰的错误信息。

generateKeyBetween(prevSortKey, nextSortKey)prevSortKey >= nextSortKey(两者均非 null 时)会抛出异常。虽然外层 ResultAsync.fromPromise 会捕获并转为 DatabaseError,但此时错误信息会是 fractional-indexing 库的内部错误,对排查不太友好。建议在调用前添加简单的前置校验,抛出更明确的 ServiceError

♻️ 添加前置校验
 			// 生成新的 sort_key(在 prevSortKey 和 nextSortKey 之间)
+			if (prevSortKey !== null && nextSortKey !== null && prevSortKey >= nextSortKey) {
+				throw new ServiceError(
+					`无效的排序键范围: prevSortKey="${prevSortKey}" >= nextSortKey="${nextSortKey}"`,
+				)
+			}
 			const newSortKey = generateKeyBetween(prevSortKey, nextSortKey)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/mobile/src/lib/services/playlistService.ts` around lines 396 - 453,
Inside reorderSingleLocalPlaylistTrack, add a precondition check before calling
generateKeyBetween: if both prevSortKey and nextSortKey are non-null and
prevSortKey >= nextSortKey, throw a ServiceError with a clear message (e.g.
'Invalid sort keys: prevSortKey must be less than nextSortKey') so callers get a
meaningful ServiceError instead of a low-level fractional-indexing exception;
keep the check near the destructuring of payload (referencing prevSortKey,
nextSortKey, generateKeyBetween, and ServiceError) so ResultAsync.fromPromise
still wraps other DB errors as before.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@apps/mobile/src/app/playlist/local/`[id].tsx:
- Around line 157-173: batchAddTracksModalPayloads currently returns [] early
when any selected trackId is missing and rebuilds the flat list on every
iteration; instead, build a single lookup Map from playlistData?.pages (flatten
pages->tracks once) keyed by track.id, then iterate selected and for each
trackId do a Map.get—if missing, skip that id (do not return []) so previously
collected payloads are preserved; when pushing payloads keep the same shape ({
track: { ...track, artistId: track.artist?.id }, artist: track.artist! }) and
ensure you handle undefined artist safely.

---

Duplicate comments:
In `@apps/mobile/CHANGELOG.md`:
- Line 15: The changelog entry "优化无网状态下的本地播放列表显示逻辑,高亮已下载和自动缓存的歌曲。" ends with a
Chinese period unlike other entries; remove the trailing "。" so it matches the
file's style (no terminal punctuation) and keep the rest of the text unchanged.

In `@apps/mobile/src/app/playlist/local/`[id].tsx:
- Around line 126-155: playableOfflineKeys is currently computed from
allLoadedTracks (only loaded pages) causing unloaded-but-cached/downloaded
tracks to be excluded; change the useMemo to iterate the complete track list
used by playAll (the full playlist/tracks variable that contains all items)
instead of allLoadedTracks, still calling getInternalPlayUri for non-local
tracks, querying Orpheus.getLruCachedUris for URIs and checking
downloadStatus[...] === DownloadState.COMPLETED, and update the dependency array
to include that full tracks variable and downloadStatus so offline-playable keys
cover every track in the playlist.
- Around line 329-347: The auto-scroll interval stored in autoScrollRef isn't
cleared on component unmount; add a useEffect that runs once and returns a
cleanup which calls stopAutoScroll to clear any existing interval. Reference the
existing stopAutoScroll and autoScrollRef/startAutoScroll functions: import or
use stopAutoScroll in the effect's cleanup and include it in the effect
dependencies so the interval is always cleared when the component unmounts or
when stopAutoScroll changes.
- Around line 415-469: The drag handler uses finalPlaylistData visual indexes to
read allLoadedSortKeys directly, which is wrong when finalPlaylistData is a
filtered subset; update handleDragEnd to map the visual neighbor(s) to their
positions in the full list before picking sort keys: determine the neighbor
track IDs from finalPlaylistData at the computed clamped/neighbor visual
positions, find each neighbor's index in allLoadedTracks (e.g. via findIndex by
id), then read prevSortKey/nextSortKey from allLoadedSortKeys at those full-list
indexes and pass them to reorderTrack (use draggingRef.current.trackId,
insertAfterIndexRef.current, finalPlaylistData, allLoadedTracks,
allLoadedSortKeys, and reorderTrack to implement this mapping).

---

Nitpick comments:
In `@apps/mobile/src/app/playlist/local/`[id].tsx:
- Around line 101-115: Extract the repeated type assertion into a local variable
(e.g., const pages = playlistData?.pages as Array<{ tracks: Track[]; sortKeys:
string[] }> ) and then replace the three occurrences inside the allLoadedTracks
useMemo, allLoadedSortKeys useMemo and the IIFE at the later block with that
variable; this keeps the type assertion in one place while the existing symbols
(playlistData, allLoadedTracks, allLoadedSortKeys, and the IIFE that reads
pages) continue to work unchanged.

In `@apps/mobile/src/hooks/app/useFastMigrations.ts`:
- Around line 65-78: The current migrateSortKeys logic iterates and calls
expoDb.runSync per row (inside expoDb.withTransactionSync), causing UI jank for
large playlists; change it to produce and execute a single batched UPDATE per
playlist: for each playlistId in the grouped loop use generateKeyBetween to
compute all sort keys into arrays, then build one SQL update using a CASE WHEN
mapping track_id -> sort_key with a WHERE playlist_id = ? AND track_id IN (...)
(or a single parameterized multi-row UPDATE), and call expoDb.runSync once per
playlist (still inside expoDb.withTransactionSync) instead of calling
expoDb.runSync for every track_id. Ensure you reference generateKeyBetween,
expoDb.withTransactionSync, and expoDb.runSync when making the change.

In `@apps/mobile/src/lib/db/schema.ts`:
- Around line 132-137: 移除冗余的单列索引 playlist_tracks_playlist_idx:在定义表索引的数组中删除
index('playlist_tracks_playlist_idx').on(table.playlistId) 因为已存在的复合索引
index('playlist_tracks_sort_key_idx').on(table.playlistId, table.sortKey)
覆盖了所有基于 table.playlistId 的查询,保留 primaryKey(...) 和 playlist_tracks_sort_key_idx
即可减少写入时的索引维护开销。

In `@apps/mobile/src/lib/services/playlistService.ts`:
- Around line 396-453: Inside reorderSingleLocalPlaylistTrack, add a
precondition check before calling generateKeyBetween: if both prevSortKey and
nextSortKey are non-null and prevSortKey >= nextSortKey, throw a ServiceError
with a clear message (e.g. 'Invalid sort keys: prevSortKey must be less than
nextSortKey') so callers get a meaningful ServiceError instead of a low-level
fractional-indexing exception; keep the check near the destructuring of payload
(referencing prevSortKey, nextSortKey, generateKeyBetween, and ServiceError) so
ResultAsync.fromPromise still wraps other DB errors as before.

ℹ️ Review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 147d0ed and 407905f.

⛔ Files ignored due to path filters (1)
  • pnpm-lock.yaml is excluded by !**/pnpm-lock.yaml
📒 Files selected for processing (14)
  • apps/mobile/CHANGELOG.md
  • apps/mobile/drizzle/0013_jittery_randall.sql
  • apps/mobile/drizzle/meta/0013_snapshot.json
  • apps/mobile/drizzle/meta/_journal.json
  • apps/mobile/drizzle/migrations.js
  • apps/mobile/package.json
  • apps/mobile/src/app/playlist/local/[id].tsx
  • apps/mobile/src/hooks/app/useFastMigrations.ts
  • apps/mobile/src/hooks/mutations/db/playlist.ts
  • apps/mobile/src/hooks/queries/db/playlist.ts
  • apps/mobile/src/lib/db/schema.ts
  • apps/mobile/src/lib/services/playlistService.ts
  • apps/mobile/src/types/services/playlist.ts
  • apps/mobile/src/types/storage.ts
✅ Files skipped from review due to trivial changes (1)
  • apps/mobile/drizzle/meta/0013_snapshot.json

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

Caution

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

⚠️ Outside diff range comments (2)
apps/mobile/src/lib/services/playlistService.ts (1)

397-447: ⚠️ Potential issue | 🟠 Major

缺少更新结果校验,可能对不存在的 track 仍返回成功。
建议在更新后校验受影响行数,未命中时抛出 createTrackNotInPlaylist

🔧 建议修复
-				await Sentry.startSpan(
+				const updated = await Sentry.startSpan(
 					{ name: 'db:update:playlistTrack:sortKey', op: 'db' },
 					() =>
 						this.db
 							.update(schema.playlistTracks)
 							.set({ sortKey: newSortKey })
 							.where(
 								and(
 									eq(schema.playlistTracks.playlistId, playlistId),
 									eq(schema.playlistTracks.trackId, trackId),
 								),
 							),
 				)
+				// drizzle sqlite 可用 returning();若无 returning,可用 changes() 或额外查询
+				if (updated.length === 0) {
+					throw createTrackNotInPlaylist(trackId, playlistId)
+				}

Also applies to: 460-462

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/mobile/src/lib/services/playlistService.ts` around lines 397 - 447, In
reorderSingleLocalPlaylistTrack, after calling
this.db.update(schema.playlistTracks).set({ sortKey: newSortKey }).where(...)
you must check the update result (affected/rowCount) and if no rows were updated
throw createTrackNotInPlaylist(playlistId, trackId); do the same for the other
update branch that updates playlistTracks (the second DB update call around the
later update path) so the method does not return success when the track was not
found/updated.
apps/mobile/src/features/playlist/local/components/LocalTrackList.tsx (1)

312-342: ⚠️ Potential issue | 🔴 Critical

确保对 onDragStartonDragUpdateonDragEnd 回调使用 useCallback 包裹。

三个拖拽回调函数(handleDragStartupdateDragPositionhandleDragEnd)目前未被 useCallback 包裹,导致每次父组件渲染时都会创建新的函数引用。这些函数随后被传递至 LocalTrackList 并包含在 extraData 的 useMemo 依赖数组中,每次引用变化都会导致整个列表不必要的重渲染。建议在 LocalPlaylistPage 中对这些回调进行 useCallback 包裹,以维持稳定的函数引用。

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/mobile/src/features/playlist/local/components/LocalTrackList.tsx` around
lines 312 - 342, The three drag callbacks (handleDragStart, updateDragPosition,
handleDragEnd) are not memoized, causing unstable references in the extraData
useMemo (onDragStart, onDragUpdate, onDragEnd) and unnecessary list re-renders;
wrap each of those functions in useCallback in the LocalPlaylistPage where they
are defined, include only the true dependencies they use (state setters, refs or
props) so the callbacks remain stable across renders, and then pass those
memoized callbacks into LocalTrackList (onDragStart/onDragUpdate/onDragEnd) so
extraData's dependencies stop changing on every render.
♻️ Duplicate comments (1)
apps/mobile/src/features/playlist/local/components/LocalTrackList.tsx (1)

18-18: MD3Theme 已改用公开 API 导入,之前的审查意见已解决。

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/mobile/src/features/playlist/local/components/LocalTrackList.tsx` at
line 18, Update the MD3Theme type import in LocalTrackList.tsx to use the public
API: ensure any previous internal/private import is replaced with import type {
MD3Theme } from 'react-native-paper' (verify the symbol MD3Theme is referenced
only from that public import and remove any duplicate or stale imports).
🧹 Nitpick comments (6)
packages/orpheus/android/src/main/java/expo/modules/orpheus/util/GlideBitmapLoader.kt (1)

48-55: 建议回收 Glide 返回的原始 Bitmap,避免内存泄漏风险加剧。

submit(512, 512).get() 返回的 glideBitmap.copy() 后未调用 recycle(),同时对应的 FutureTarget 也未被存储,无法通过 Glide.with(context).clear(target) 释放。启用磁盘缓存后,该路径的调用频次会增加,累积的未回收 Bitmap 可能加大内存压力。

♻️ 建议修复:回收原始 Bitmap
             if (glideBitmap != null && !glideBitmap.isRecycled) {
                 val safeBitmap = glideBitmap.copy(Bitmap.Config.ARGB_8888, false)
+                glideBitmap.recycle()
                 return@submit safeBitmap
             } else {
                 throw IllegalStateException("Bitmap load failed or recycled")
             }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In
`@packages/orpheus/android/src/main/java/expo/modules/orpheus/util/GlideBitmapLoader.kt`
around lines 48 - 55, The original Glide bitmap (glideBitmap) returned by
submit(512, 512).get() is copied to safeBitmap but never recycled or the
FutureTarget cleared, risking memory leaks; update the code that calls
submit(...) to store the FutureTarget (e.g., futureTarget), call
futureTarget.get() to obtain glideBitmap, create safeBitmap via
glideBitmap.copy(...), then immediately recycle the original glideBitmap
(glideBitmap.recycle()) and clear the FutureTarget with
Glide.with(context).clear(futureTarget) in a finally block so both the bitmap
and target are released even on error; make these changes around the current
glideBitmap/safeBitmap logic in GlideBitmapLoader.kt.
apps/mobile/src/features/library/multipage/MultiPageVideosList.tsx (1)

108-108: 底部内边距的魔法数字可以考虑提取为共享常量。

本 PR 中多个列表组件(MultiPageVideosListFavoriteFolderListCollectionList等)都将 paddingBottom 从 70 改为 90。建议将 9010 提取为一个共享常量(如 BOTTOM_PADDING_WITH_TRACK / BOTTOM_PADDING_DEFAULT),便于后续统一调整,避免散落在各文件中的魔法数字不一致。

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/mobile/src/features/library/multipage/MultiPageVideosList.tsx` at line
108, Multiple list components (e.g., MultiPageVideosList, FavoriteFolderList,
CollectionList) use magic numbers for contentContainerStyle.paddingBottom (90
and 10); extract these into shared constants (e.g., BOTTOM_PADDING_WITH_TRACK
and BOTTOM_PADDING_DEFAULT) in a common constants module, export them, replace
hard-coded 90/10 in MultiPageVideosList and the other list components with the
new constants, and add/update imports where needed so all lists reference the
same values for unified future adjustments.
apps/mobile/src/features/playlist/local/components/LocalTrackList.tsx (1)

182-191: 拖拽过程中 insertAfterIndex 频繁变化可能导致全列表重渲染。

insertAfterIndex 作为 extraData 的一部分,每次变化都会触发 FlashList 重新渲染所有可见项。在拖拽手势进行中,insertAfterIndex 会随手指移动高频更新,可能导致列表卡顿。

更高性能的方案是将插入指示线作为 overlay 覆盖在列表上方(例如使用 Reanimated shared value 控制绝对定位的 View),而非嵌入到每个列表项的渲染逻辑中。这样仅需移动一个 overlay 元素,而非重渲染所有可见项。

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/mobile/src/features/playlist/local/components/LocalTrackList.tsx` around
lines 182 - 191, The insert indicator is causing FlashList to re-render all
visible items because insertAfterIndex is included in extraData and updates
frequently during drag; refactor LocalTrackList to remove the per-item
conditional that renders the indicator (the block checking insertAfterIndex ===
index inside the item render) and instead render a single absolute-positioned
overlay indicator above the FlashList (controlled by a shared value or state
driven by the drag handlers). Keep FlashList.extraData free of the rapidly
changing insertAfterIndex, update only stable data there, and drive the overlay
position from the drag gesture (e.g., a Reanimated shared value or a single
piece of state updated at lower frequency) so the list items no longer re-render
on each drag movement.
apps/mobile/src/app/playlist/local/[id].tsx (2)

107-136: 建议用 useMemo 缓存离线可播集合,避免每次渲染重复计算
Orpheus.getLruCachedUrisSet 构造在每次渲染都会执行,曲目多时会放大开销。可以用 useMemoallLoadedTracksdownloadStatus 作为依赖。

♻️ 建议改法
-	const playableOfflineKeys = (() => {
+	const playableOfflineKeys = useMemo(() => {
 		if (!allLoadedTracks.length) return new Set<string>()
@@
 		return keys
-	})()
+	}, [allLoadedTracks, downloadStatus])
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/mobile/src/app/playlist/local/`[id].tsx around lines 107 - 136, The
current inline IIFE computes playableOfflineKeys on every render which
repeatedly calls Orpheus.getLruCachedUris and constructs Sets; wrap this logic
in React's useMemo to memoize the Set and avoid recomputation, using
allLoadedTracks and downloadStatus as dependencies (reference the
playableOfflineKeys constant and the internal use of Orpheus.getLruCachedUris,
getInternalPlayUri, and DownloadState.COMPLETED) so the memo only recalculates
when those inputs change.

51-57: 拖拽高度常量可能在字体缩放/主题变化下失准
SELECT_MODE_ITEM_HEIGHT 固定为 69 时,系统字体缩放或样式变更可能导致插入线与幽灵项错位。建议基于实际 item 的 onLayout 动态测量或集中管理该高度。

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/mobile/src/app/playlist/local/`[id].tsx around lines 51 - 57,
SELECT_MODE_ITEM_HEIGHT is a hardcoded 69 which can desync when fonts/themes
change; replace the fixed constant by measuring the actual item height at
runtime and using that value where needed: add an onLayout handler on the
playlist item component to capture its height (store in state or a centralized
context/manager) and use that measured value instead of SELECT_MODE_ITEM_HEIGHT
for insertion-line placement and auto-scroll calculations
(EDGE_ZONE/SCROLL_SPEED logic should read the dynamic height where relevant);
ensure a sensible fallback/default if measurement isn't available yet.
apps/mobile/src/app/settings/appearance.tsx (1)

16-184: 可考虑抽离设置页通用布局以减少重复
多个设置页都在做相同的 NowPlayingBar 容器与 padding 计算,建议封装成共享布局或 HOC,便于统一维护 bar 高度与 inset 逻辑。

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/mobile/src/app/settings/appearance.tsx` around lines 16 - 184, This file
duplicates NowPlayingBar placement and inset/padding logic (see NowPlayingBar
usage, styles.nowPlayingBarContainer, ScrollView contentContainerStyle
paddingBottom calc using insets and useCurrentTrack()); extract a shared
SettingsLayout component (or HOC) that wraps a ScrollView plus the NowPlayingBar
container and exposes props for children and optional haveTrack, so other
settings pages can use SettingsLayout({ children, haveTrack }) to centralize the
insets/padding calculation and bar rendering; update AppearanceSettingsPage to
use the new SettingsLayout and remove the duplicated NowPlayingBar,
paddingBottom logic, and styles related to nowPlaying bar.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@apps/mobile/src/app/playlist/local/`[id].tsx:
- Around line 405-429: The reorder logic can place a track at the true list
bottom when not all pages are loaded because allLoadedSortKeys only contains
keys for loaded pages; update the handler that computes prevSortKey/nextSortKey
(the block using targetVisualIndex, trackIndex, clamped, allLoadedSortKeys) to
first detect incomplete pagination (e.g. compare finalPlaylistData length vs
total count) and either (a) disable/ignore the drag action (prevent calling
reorderTrack) when not all pages are loaded, or (b) fetch the missing boundary
sort_keys before computing prevSortKey/nextSortKey and only call
reorderTrack(playlistId: Number(id), trackId, prevSortKey, nextSortKey) after
you have valid adjacent keys; reference the variables/functions
allLoadedSortKeys, targetVisualIndex, trackIndex, finalPlaylistData,
reorderTrack, id and trackId when implementing the guard/fetch.

---

Outside diff comments:
In `@apps/mobile/src/features/playlist/local/components/LocalTrackList.tsx`:
- Around line 312-342: The three drag callbacks (handleDragStart,
updateDragPosition, handleDragEnd) are not memoized, causing unstable references
in the extraData useMemo (onDragStart, onDragUpdate, onDragEnd) and unnecessary
list re-renders; wrap each of those functions in useCallback in the
LocalPlaylistPage where they are defined, include only the true dependencies
they use (state setters, refs or props) so the callbacks remain stable across
renders, and then pass those memoized callbacks into LocalTrackList
(onDragStart/onDragUpdate/onDragEnd) so extraData's dependencies stop changing
on every render.

In `@apps/mobile/src/lib/services/playlistService.ts`:
- Around line 397-447: In reorderSingleLocalPlaylistTrack, after calling
this.db.update(schema.playlistTracks).set({ sortKey: newSortKey }).where(...)
you must check the update result (affected/rowCount) and if no rows were updated
throw createTrackNotInPlaylist(playlistId, trackId); do the same for the other
update branch that updates playlistTracks (the second DB update call around the
later update path) so the method does not return success when the track was not
found/updated.

---

Duplicate comments:
In `@apps/mobile/src/features/playlist/local/components/LocalTrackList.tsx`:
- Line 18: Update the MD3Theme type import in LocalTrackList.tsx to use the
public API: ensure any previous internal/private import is replaced with import
type { MD3Theme } from 'react-native-paper' (verify the symbol MD3Theme is
referenced only from that public import and remove any duplicate or stale
imports).

---

Nitpick comments:
In `@apps/mobile/src/app/playlist/local/`[id].tsx:
- Around line 107-136: The current inline IIFE computes playableOfflineKeys on
every render which repeatedly calls Orpheus.getLruCachedUris and constructs
Sets; wrap this logic in React's useMemo to memoize the Set and avoid
recomputation, using allLoadedTracks and downloadStatus as dependencies
(reference the playableOfflineKeys constant and the internal use of
Orpheus.getLruCachedUris, getInternalPlayUri, and DownloadState.COMPLETED) so
the memo only recalculates when those inputs change.
- Around line 51-57: SELECT_MODE_ITEM_HEIGHT is a hardcoded 69 which can desync
when fonts/themes change; replace the fixed constant by measuring the actual
item height at runtime and using that value where needed: add an onLayout
handler on the playlist item component to capture its height (store in state or
a centralized context/manager) and use that measured value instead of
SELECT_MODE_ITEM_HEIGHT for insertion-line placement and auto-scroll
calculations (EDGE_ZONE/SCROLL_SPEED logic should read the dynamic height where
relevant); ensure a sensible fallback/default if measurement isn't available
yet.

In `@apps/mobile/src/app/settings/appearance.tsx`:
- Around line 16-184: This file duplicates NowPlayingBar placement and
inset/padding logic (see NowPlayingBar usage, styles.nowPlayingBarContainer,
ScrollView contentContainerStyle paddingBottom calc using insets and
useCurrentTrack()); extract a shared SettingsLayout component (or HOC) that
wraps a ScrollView plus the NowPlayingBar container and exposes props for
children and optional haveTrack, so other settings pages can use
SettingsLayout({ children, haveTrack }) to centralize the insets/padding
calculation and bar rendering; update AppearanceSettingsPage to use the new
SettingsLayout and remove the duplicated NowPlayingBar, paddingBottom logic, and
styles related to nowPlaying bar.

In `@apps/mobile/src/features/library/multipage/MultiPageVideosList.tsx`:
- Line 108: Multiple list components (e.g., MultiPageVideosList,
FavoriteFolderList, CollectionList) use magic numbers for
contentContainerStyle.paddingBottom (90 and 10); extract these into shared
constants (e.g., BOTTOM_PADDING_WITH_TRACK and BOTTOM_PADDING_DEFAULT) in a
common constants module, export them, replace hard-coded 90/10 in
MultiPageVideosList and the other list components with the new constants, and
add/update imports where needed so all lists reference the same values for
unified future adjustments.

In `@apps/mobile/src/features/playlist/local/components/LocalTrackList.tsx`:
- Around line 182-191: The insert indicator is causing FlashList to re-render
all visible items because insertAfterIndex is included in extraData and updates
frequently during drag; refactor LocalTrackList to remove the per-item
conditional that renders the indicator (the block checking insertAfterIndex ===
index inside the item render) and instead render a single absolute-positioned
overlay indicator above the FlashList (controlled by a shared value or state
driven by the drag handlers). Keep FlashList.extraData free of the rapidly
changing insertAfterIndex, update only stable data there, and drive the overlay
position from the drag gesture (e.g., a Reanimated shared value or a single
piece of state updated at lower frequency) so the list items no longer re-render
on each drag movement.

In
`@packages/orpheus/android/src/main/java/expo/modules/orpheus/util/GlideBitmapLoader.kt`:
- Around line 48-55: The original Glide bitmap (glideBitmap) returned by
submit(512, 512).get() is copied to safeBitmap but never recycled or the
FutureTarget cleared, risking memory leaks; update the code that calls
submit(...) to store the FutureTarget (e.g., futureTarget), call
futureTarget.get() to obtain glideBitmap, create safeBitmap via
glideBitmap.copy(...), then immediately recycle the original glideBitmap
(glideBitmap.recycle()) and clear the FutureTarget with
Glide.with(context).clear(futureTarget) in a finally block so both the bitmap
and target are released even on error; make these changes around the current
glideBitmap/safeBitmap logic in GlideBitmapLoader.kt.

ℹ️ Review info

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 407905f and 85c4d2b.

📒 Files selected for processing (22)
  • apps/mobile/CHANGELOG.md
  • apps/mobile/drizzle/0014_flippant_sebastian_shaw.sql
  • apps/mobile/drizzle/meta/0014_snapshot.json
  • apps/mobile/drizzle/meta/_journal.json
  • apps/mobile/drizzle/migrations.js
  • apps/mobile/src/app/(tabs)/_layout.tsx
  • apps/mobile/src/app/(tabs)/settings/_layout.tsx
  • apps/mobile/src/app/_layout.tsx
  • apps/mobile/src/app/playlist/local/[id].tsx
  • apps/mobile/src/app/settings/appearance.tsx
  • apps/mobile/src/app/settings/donate.tsx
  • apps/mobile/src/app/settings/general.tsx
  • apps/mobile/src/app/settings/lyrics.tsx
  • apps/mobile/src/app/settings/playback.tsx
  • apps/mobile/src/features/library/collection/CollectionList.tsx
  • apps/mobile/src/features/library/favorite/FavoriteFolderList.tsx
  • apps/mobile/src/features/library/local/LocalPlaylistList.tsx
  • apps/mobile/src/features/library/multipage/MultiPageVideosList.tsx
  • apps/mobile/src/features/playlist/local/components/LocalTrackList.tsx
  • apps/mobile/src/lib/db/schema.ts
  • apps/mobile/src/lib/services/playlistService.ts
  • packages/orpheus/android/src/main/java/expo/modules/orpheus/util/GlideBitmapLoader.kt
💤 Files with no reviewable changes (1)
  • apps/mobile/src/app/(tabs)/settings/_layout.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
  • apps/mobile/drizzle/migrations.js

Comment thread apps/mobile/src/app/playlist/local/[id].tsx
@roitium roitium merged commit ac98923 into dev Feb 24, 2026
6 checks passed
This was referenced Feb 27, 2026
@roitium roitium deleted the feat/1 branch February 28, 2026 12:32
@coderabbitai coderabbitai Bot mentioned this pull request Mar 24, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant