You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
feat(bilibili): 支持把网易云歌曲 / 专辑 / 歌单绑定到 B 站音源 - #631
The reason will be displayed to describe this comment to others. Learn more.
Register record 17 in the metadata scanner
When an archive contains a binding, this now admits kind 17 and payload() then calls SyncArchiveMetadataBudget.inspect, but SyncArchiveMetadataWire.descriptors still has only the 16 serializers for kinds 1–16 and indexes it with descriptors[kind - 1]. Consequently, the next read of any uploaded archive containing biliCollectionBindings throws an IndexOutOfBoundsException; add the binding descriptor (and its nested track descriptor) before accepting/writing this kind.
The reason will be displayed to describe this comment to others. Learn more.
Fixed in b7a09ed: appended SyncBiliCollectionBinding.serializer().descriptor to SyncArchiveMetadataWire.descriptors (index 16 for kind 17). The nested track descriptor is picked up automatically by nested() since field 8 is a list of a class. Added SyncArchiveBiliCollectionBindingTest, which writes an archive containing an active binding plus a tombstone and reads it back through SyncArchiveRecords.read (that path runs the metadata scanner, so it reproduces the out-of-bounds failure).
The reason will be displayed to describe this comment to others. Learn more.
Version the new archive record before publishing it
On a sync target shared with an older app build, writing kind 17 under the unchanged V4 protocol/schema is incompatible: the parent reader hard-codes kind in 1..16, while SyncArchiveV4Format.PROTOCOL/SCHEMA and CURRENT_PROTOCOL_VERSION remain 4/1/4. Once an upgraded device uploads a binding, every older device rejects the entire archive instead of ignoring the new section, so this record needs a protocol/schema bump and upgrade gate (or a backward-compatible encoding).
The reason will be displayed to describe this comment to others. Learn more.
Checked this one carefully and deliberately did not bump the archive version. SyncArchiveV4Format.readManifest requires value.protocol == PROTOCOL && value.schema == SCHEMA exactly, and the repo ships frozen v3/v4 fixtures (SyncArchiveVersionMigrationTest, /sync/archive/v4-frozen). Raising PROTOCOL/SCHEMA makes every existing v4 archive unreadable, so it needs a real format migration (accept N-1 manifests, or a negotiation field) plus the CURRENT_PROTOCOL_VERSION upgrade gate; that is a cross-cutting change I would rather not fold into this feature PR.
What the change does instead: the record is additive within V4, so failures stay on the error path — an older build hits require(kind in 1..16) and the archive read returns a failure (SyncBackup ... invalid message), it does not corrupt remote or local data. The limitation is now recorded in the PR's Risks & Compatibility section so the maintainer can decide whether a protocol bump ships with this release.
The reason will be displayed to describe this comment to others. Learn more.
Persist a tombstone when removing a synced binding
When the binding has already been uploaded or exists on another device, this physical delete makes AndroidSyncSnapshotBuilder.albumBindings() emit no local candidate for the key; the union/latest-wins merge therefore retains the remote active row, and applyAlbumBindings() restores it via replaceAll on the next sync. The sync model and merge policy already support isDeleted records, so unbinding must retain or otherwise emit a timestamped tombstone rather than removing all evidence of the deletion.
The reason will be displayed to describe this comment to others. Learn more.
Fixed in b7a09ed. remove() now upserts a timestamped tombstone instead of deleteByKey; tombstones are stored with an empty bvid (no schema change), loadFromDatabase/UI/get()/all() filter them out, while snapshot() — the only consumer is AndroidSyncSnapshotBuilder.albumBindings() — includes them so isDeleted = true reaches SyncBiliCollectionBindingMergePolicy. Tombstones coming back from a remote are persisted too, so they keep suppressing the stale row. Covered by BiliCollectionBindingRepositoryTest (unbind → tombstone only in snapshot; tombstone survives replaceAll; re-bind replaces it).
The reason will be displayed to describe this comment to others. Learn more.
Enforce unique page assignments before saving
When manually correcting a multi-song match, every row offers the same pages and this update permits two different songs to select the same cid. Both rows then appear matched and save reports success, but BiliCollectionBinding.normalized() applies distinctBy { it.cid }, silently discarding one song's track; clear the previous owner or reject the duplicate selection (or preserve both mappings) before persisting.
The reason will be displayed to describe this comment to others. Learn more.
Fixed in b7a09ed by making the selection exclusive: picking a page for one song now drops that same cid from any other song's assignment (assignments.filterValues { it != page.cid } + (song.id to page.cid)). The previous owner's row immediately shows as unmatched, so nothing is silently discarded at save time.
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.
新增 B 站单曲、专辑和歌单分集绑定功能，包含本地存储、视频匹配、绑定界面和播放解析。绑定数据接入同步归档、合并及本地应用流程。新增网易云自动切源音质探测和 B 站绑定优先播放设置，并调整播放来源标识解析。
Merge Risk: Low · previous review b7a09
A concurrent sync can rarely overwrite a newly saved binding, and the keyboard’s search and done keys do not perform their advertised actions. The changes are mergeable with owner awareness, though the binding commit should be made atomic.
Pre-merge checks: 4 · 1
Failed checks: 1 warning
Check name
Status
Explanation
Resolution
Docstring Coverage
Docstring coverage is 35.85% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documenta…
Write docstrings for the functions missing them to satisfy the coverage threshold.
Passed checks: 4 passed
Check name
Status
Explanation
Description Check
Check skipped - CodeRabbit’s high-level summary is enabled.
Title check
标题准确概括了本次变更的主要功能：将网易云歌曲、专辑和歌单绑定到 B 站音源。
Linked Issues check
Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check
Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage
Explanation
Docstring coverage is 35.85% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 53 functions across 54 files.
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
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.
Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.
The reason will be displayed to describe this comment to others. Learn more.
Fixed in b7a09ed — same fix as the Codex finding above. Unbind writes a tombstone (empty bvid, fresh updatedAt), snapshot() now emits isDeleted = true records for the sync payload, playback/UI reads filter tombstones, and remote tombstones are persisted on apply. I also removed the now-incorrect applyAlbumBindings comment that claimed there were no deletion tombstones.
--- "a/modules/model/src/main/java/moe/ouom/neriplayer/data/model/bilibili/binding/BiliCollectionBinding.kt"+++ "b/modules/model/src/main/java/moe/ouom/neriplayer/data/model/bilibili/binding/BiliCollectionBinding.kt"@@ -42,15 +42,19 @@
val tracks: List<BiliCollectionBindingTrack> = emptyList(),
val updatedAt: Long = 0L
) {
fun trackFor(song: SongItem): BiliCollectionBindingTrack? {
if (tracks.isEmpty()) return null
if (song.id > 0L) {
tracks.firstOrNull { it.songId > 0L && it.songId == song.id }?.let { return it }
}
val songName = normalizeBiliBindingText(song.originalName ?: song.name)
if (songName.isEmpty()) return null
- return tracks.firstOrNull { normalizeBiliBindingText(it.songName) == songName }+ // 双方都有 id 时 id 不同即视为不同歌曲，避免同名歌曲串播+ return tracks.firstOrNull {+ (song.id <= 0L || it.songId <= 0L) &&+ normalizeBiliBindingText(it.songName) == songName+ }
}
/**
* 这次保存或删除的绑定是否牵涉这首歌：单曲键、所属专辑键或当前播放歌单键命中，
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.
The reason will be displayed to describe this comment to others. Learn more.
Fixed in b7a09ed, using the suggested shape: when both sides have a positive id, only the id match counts; the name fallback is limited to rows where one side lacks an id ((song.id <= 0L || track.songId <= 0L) && normalizedNameMatch). Added same name with a different id never borrows the binding to BiliCollectionBindingAffectsSongTest so a same-title cover cannot hijack the bound Bilibili page.
applyAlbumBindings now passes expectedMutationVersion into replaceAll, which compares it against the local sync mutation version inside the repository mutex and returns false (skipping the replace and logging) when a local binding write happened after the snapshot — matching BiliVideoSkipRepository.replaceFromSyncIfUnchanged.
BiliCollectionBindingDao gained a @Transaction suspend fun replaceAll(...), so delete + insert can no longer leave an empty table.
Added a regression test (stale sync snapshot does not overwrite a newer local change).
The reason will be displayed to describe this comment to others. Learn more.
Fixed in b7a09ed: descriptor appended so descriptors[16] is SyncBiliCollectionBinding.serializer().descriptor, plus a new SyncArchiveBiliCollectionBindingTest that round-trips kind 17 through the archive reader (the reader path runs SyncArchiveMetadataBudget.inspect, so the test exercises the scanner that used to blow up).
The reason will be displayed to describe this comment to others. Learn more.
Mark binding mutations before releasing the lock
With the production BiliCollectionBindingRepositoryProvider, onChanged() is the call that advances SecureTokenStorage's sync mutation version. Because this call happens after mutex.withLock returns, a replaceAll() already waiting on the mutex can acquire it in between, still observe the snapshot's old version, and overwrite the just-saved row; the subsequent callback then marks a mutation for data that has already been lost. Mark the mutation while the write is still protected (the same ordering is needed in remove).
The reason will be displayed to describe this comment to others. Learn more.
Fixed in 4eddf0d. The repository now takes a separate markMutation callback and calls it insidemutex.withLock (right after the DB write and cache update) in both save and remove; onChanged only schedules the sync workers and still runs after the lock. A replaceAll that was waiting on the mutex therefore observes the bumped version and skips, so the just-saved row cannot be wiped. Regression test: local writes advance the sync mutation version before releasing the lock.
The reason will be displayed to describe this comment to others. Learn more.
Resolve bindings before reusing the logical-song cache
When a NetEase song already has a complete ExoPlayer cache entry (or a generic URL prefetch), resolveSongUrl returns that result before it ever calls getNeteaseSongUrl, so this new binding lookup is unreachable. Saving a binding or enabling the priority switch refreshes NetEase with forceRefresh=false, leaving the cached NetEase source playing despite the setting's promise that bound parts play directly. Resolve the binding before those logical-song cache fast paths or make the binding/source part of cache invalidation/keying.
The reason will be displayed to describe this comment to others. Learn more.
Fixed in 4eddf0d, taking the "resolve the binding before the cache fast paths" option. resolveSongUrl now attempts tryResolveNeteaseAlbumBindingSource right after the direct-stream shortcut, i.e. before the complete-ExoPlayer-cache branch and before consumeGenericUrlPrefetch; a successful bound resolution returns immediately, so a cached NetEase copy can no longer win. When the binding is absent or its stream cannot be resolved (expired/private video, offline), the call returns null and the pre-existing cache/prefetch/network path runs unchanged, so offline playback of already-cached audio is not lost. Skipped when shouldDeferNeteaseAlternateSources is set, so the Listen Together authoritative source still takes precedence. Local downloaded files (checkLocalCache) still win, since that is an explicit offline choice.
The reason will be displayed to describe this comment to others. Learn more.
Report Bilibili only when that stream was selected
When binding priority is disabled, or when getAudioWithDecision fails for an expired/private video and the resolver falls back, playbackAudioSource is correctly NETEASE but this branch rewrites it to Bilibili merely because a row exists. The Now Playing badge then reports a source that is not being played; only override an unknown/stale value while a bound Bilibili resolution is actually active (or trust the BILIBILI audio info emitted by the resolver).
The reason will be displayed to describe this comment to others. Learn more.
Fixed in 4eddf0d. The badge now only falls back to Bilibili while playbackAudioSource == null, i.e. when there is no measured source yet and the NetEase label would just be a guess from song metadata. Once a concrete source exists it wins: bound playback emits PlaybackAudioSource.BILIBILI from buildBiliPlaybackAudioInfo, so a genuinely bound song still shows Bilibili, while a NetEase fallback (priority off or dead video) truthfully stays NetEase. Updated bili collection binding keeps netease badge when netease audio is playing and kept also overrides tag only resolution for the unresolved case.
The reason will be displayed to describe this comment to others. Learn more.
Advance deletion tombstones past the previous version
When a binding was synced from a device whose clock is ahead of this device, its existing updatedAt can be greater than System.currentTimeMillis(). This tombstone is then older than the active row, so SyncBiliCollectionBindingMergePolicy selects the active row and the next sync resurrects the binding; stamp deletions with at least a safe successor of the prior row's timestamp.
The reason will be displayed to describe this comment to others. Learn more.
Fixed in 4eddf0d: the tombstone timestamp is now maxOf(System.currentTimeMillis(), (existing?.updatedAt ?: 0L) + 1L), so a row stamped by a device whose clock runs ahead still loses to the deletion. Covered by tombstone outruns a record stamped by a device whose clock is ahead.
The reason will be displayed to describe this comment to others. Learn more.
Break equal-timestamp binding ties deterministically
When two devices save different active bindings for the same key with the same millisecond timestamp (or both records carry the default zero), merge(local, remote) passes the local record first and this fallback always returns left. Each device therefore chooses its own value and successive syncs can alternate the remote archive indefinitely. Add a stable content/device tie-break so merge is commutative.
The reason will be displayed to describe this comment to others. Learn more.
Fixed in 4eddf0d. After the timestamp and isDeleted checks, ties are broken by a stable content key (bvid, target/video titles, then cids and parts in cid order), so both devices independently pick the same record and the archive stops flipping. Added equal timestamps break ties by content so both devices agree, which asserts merge(a, b) == merge(b, a).
Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 7 remain after this review.
The reason will be displayed to describe this comment to others. Learn more.
Advance rebinding timestamps past the stored row
When the existing binding or tombstone was synced from a device whose clock is ahead, a later local edit/rebind arrives here with an updatedAt lower than current[normalized.targetKey].updatedAt; this unconditional upsert replaces the local row but leaves the remote higher-timestamp value able to win the next merge, silently reverting the user's change. Advance active saves past the cached row's timestamp, just as remove already does for tombstones.
The reason will be displayed to describe this comment to others. Learn more.
Refresh Bilibili fallback when enabling binding priority
When priority is enabled while a logical NetEase song is currently playing through the automatic Bilibili fallback, its measured audio source is BILIBILI; PlayerManagerPlaybackQualityPort.refreshCurrentSong rejects refresh jobs whose source does not match, so this NetEase-only job does nothing and the saved binding is not selected until the track is replayed or changed. Schedule a Bilibili refresh too, as the binding-change path already does.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
描述 / Description
为网易云歌曲、专辑与歌单增加「绑定到哔哩哔哩多 P 视频」的音源能力：绑定关系保存在本地 Room 数据库，并随 GitHub / WebDAV 同步；播放时优先使用已绑定的分 P，正在播放页的来源角标也会随之显示为哔哩哔哩。
类型 / Type
修复或解决的问题 / Issues Fixed or Closed by This PR
b23.tv短链、不含 BV 号，旧流程无法直接识别关键实现 / Implementation
数据与同步
bili_collection_binding表（Room 21 → 23），targetKey覆盖netease:song:<id>/netease:album:<id>/netease:playlist:<id>（无 id 时退回规范化名称），每条绑定保存视频标识与分 P 列表（tracks_json）BiliCollectionBindingTargetMigration按PRAGMA table_info分四路处理：老库只有表、只有索引、什么都没有、以及被外部清理掉目标表的情况，避免升级时no such table崩溃updatedAt取新（SyncBiliCollectionBindingMergePolicy）；本地保存或解绑后标记同步版本并排队一次延迟同步bvid表示），墓碑只参与同步、不进播放与 UI；否则「设备 A 解绑」会在下次同步被设备 B 的旧记录复活。合并时沿用策略里已有的「时间戳相同则删除优先」绑定面板
BiliCollectionBindingDialog：打开即用目标名字搜索 B 站，也可以贴 BV 号 /av号 / 分享文案，b23.tv短链会跟随跳转后还原出视频标识播放
neteaseLocalSourceFallbackEnabled）→ B 站（neteaseAutoSourceSwitchEnabled）；用户手动绑定的分 P 排在最前风险与兼容性 / Risks & Compatibility
biliAlbumBindingPriorityEnabled默认开启，但没有绑定时不改变原有解析结果kind in 1..16断言处失败（错误路径为同步失败，不会损坏数据）。没有在本次一并提升归档协议，是因为SyncArchiveV4Format的readManifest要求 protocol / schema 精确相等，提升后所有既有 v4 归档（含v4-frozen夹具）将无法读取，需要配套的格式迁移；这一点留给维护者按发版节奏决定评审反馈 / Review feedback
已在最新提交中处理：
第一轮：
SyncArchiveMetadataWire.descriptors补SyncBiliCollectionBinding描述符，并新增SyncArchiveBiliCollectionBindingTest做读写往返覆盖bvid），播放与 UI 过滤墓碑，同步快照携带墓碑expectedMutationVersion/ 整表替换非原子（CodeRabbit Major）：applyAlbumBindings比较本地写入版本，不一致时跳过并返回 false；DAO 增加@Transaction replaceAlltrackFor在双方都有 id 时只按 id 匹配第二轮：
markMutation移到仓储互斥锁内，等锁的replaceAll因此必然读到新版本而跳过resolveSongUrl在缓存快路之前先尝试绑定解析，成功即直接放 B 站分 P；解析失败再照旧走缓存与网络，不牺牲离线可用性playbackAudioSource == null）时才用绑定兜底，一旦解析出具体来源就以它为准max(当前时间, 被删记录时间 + 1)测试方式 / Testing
本机（Windows）实际执行并通过：
./gradlew :app:assembleDebug./gradlew :app:lintDebug（0 error）与:app:verifyDomainDependencies -x :app:crapToolTests、python3 tools_pub/quality/module_boundaries.py./gradlew :app:testDebugUnitTest --tests 'moe.ouom.neriplayer.ui.screen.NowPlayingScreenTest' --tests 'moe.ouom.neriplayer.ui.screen.SettingsPageTest'./gradlew :app:testDebugUnitTest --tests 'moe.ouom.neriplayer.architecture.*'（行数预算、values/values-zh/values-en键对齐）./gradlew :platform:testDebugUnitTest --tests 'moe.ouom.neriplayer.platform.bilibili.binding.*'./gradlew :model:testDebugUnitTest --tests 'moe.ouom.neriplayer.data.model.bilibili.binding.*' --tests 'moe.ouom.neriplayer.data.model.sync.SyncBiliCollectionBindingModelsTest'./gradlew :database:testDebugUnitTest --tests 'moe.ouom.neriplayer.data.local.database.migration.*'./gradlew :sync:testDebugUnitTest --tests 'moe.ouom.neriplayer.data.sync.policy.SyncBiliCollectionBindingMergePolicyTest' --tests 'moe.ouom.neriplayer.data.sync.archive.SyncArchiveBiliCollectionBindingTest'Windows 环境限制 / Windows-only limits
以下任务在本机被既有问题阻断，失败点均落在本次未改动的文件上，请以 Linux CI 结果为准：
:sync部分用例：GitHubSyncCheckpoint用File.renameTo覆盖已存在文件，Windows 上返回 false 抛IOException:local的AutoSettingsGeneratedTest中 DataStore 写入用例：settings.preferences_pb.tmp重命名失败:database的NeriUserDataDatabaseVersionTest：SQLiteCantOpenDatabaseException，连带:app:verifyCrap无法完成清单 / Checklist
其他信息 / Additional Information
绑定面板为纯 Compose 实现，未引入新依赖；B 站请求复用现有
BiliClient。