feat(music)/add conversational playback and NetEase personalization - #2571
feat(music)/add conversational playback and NetEase personalization#2571Himifox wants to merge 31 commits into
Conversation
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
Walkthrough本次变更新增结构化音乐请求解析、网易云个性化检索、用户点歌编排、候选播放派发与播放状态回报,并统一音乐占用判断、主动对话门控及多语言提示喵。 Changes音乐请求与播放链路
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested labels: Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
# Conflicts: # static/app/app-proactive.js # tests/unit/test_music_crawlers.py # utils/music_crawlers.py
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (11)
tests/unit/test_music_crawlers.py (1)
179-223: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value这个
_personalization_source_index = 5是硬算出来的魔法数字喵~测试隐式假设
NETEASE_PERSONALIZATION_SOURCE_ORDER长度为 3 且artist排在索引 2(5 % 3 == 2)。以后有人往轮换顺序里插一个来源,这条断言就会莫名其妙地红掉,而错误信息只会说results[0]['id'] != 100,超难查的喵。建议直接从常量算出目标下标:🐾 建议改法
+from utils.music_crawlers import NETEASE_PERSONALIZATION_SOURCE_ORDER + ... - crawler._personalization_source_index = 5 + crawler._personalization_source_index = ( + NETEASE_PERSONALIZATION_SOURCE_ORDER.index('artist') + )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit/test_music_crawlers.py` around lines 179 - 223, Replace the hard-coded crawler._personalization_source_index value in test_netease_personalized_recommendations_rotates_to_artist_lead with an index calculated from NETEASE_PERSONALIZATION_SOURCE_ORDER using the position of the artist source and the rotation behavior, so the test remains valid when the source order changes.tests/unit/test_proactive_phase1_pass.py (1)
59-68: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value用
pytest.parametrize替掉 for 循环会更好定位失败喵~现在这样写,第一个 directive 挂掉后面就不跑了,报错信息也看不出是哪个指令炸的。同一个 PR 里的
tests/unit/test_proactive_service_boundary.py已经用了参数化,保持一致更整齐喵。🐾 建议改法
-def test_parse_unified_phase1_preserves_music_directives(): - for directive in ( - "source:liked", - "source:daily", - "playlist:夜间循环", - "song:晴天|周杰伦", - "personalized", - ): - parsed = sr_parsing._parse_unified_phase1_result(f"[MUSIC] {directive}") - assert parsed["music_keyword"] == directive +@pytest.mark.parametrize( + "directive", + ( + "source:liked", + "source:daily", + "playlist:夜间循环", + "song:晴天|周杰伦", + "personalized", + ), +) +def test_parse_unified_phase1_preserves_music_directives(directive): + parsed = sr_parsing._parse_unified_phase1_result(f"[MUSIC] {directive}") + assert parsed["music_keyword"] == directive顺便提一句:
source:daily(全角冒号)这种变体也建议补一条,解析层对全角冒号的处理在test_proactive_service_boundary.py里是被覆盖的喵。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit/test_proactive_phase1_pass.py` around lines 59 - 68, 将 test_parse_unified_phase1_preserves_music_directives 中的 for 循环改为 pytest.parametrize,为每个 directive 独立生成测试用例并保留现有断言;同时在参数列表中补充使用全角冒号的 source:daily 变体,确保其解析结果也等于原始指令。tests/unit/test_proactive_service_boundary.py (1)
409-417: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
monkeypatch.setattr(music_playback.asyncio, "sleep", ...)打的是全局asyncio.sleep哦喵!
music_playback.asyncio并不是本地副本,它就是那个全局asyncio模块对象。所以这行等价于asyncio.sleep = finish_reply_on_sleep,在本用例期间整个进程的asyncio.sleep都被换掉了。而这个替身只接受一个位置参数,任何asyncio.sleep(0, result=x)的调用(pytest-asyncio 或其他被测协程都可能用)都会直接TypeError。monkeypatch 事后会还原,所以现在还没炸,但这是个埋着的雷喵。建议改成注入睡眠函数,或在
music_playback里暴露一个可替换的模块级别名(例如_sleep = asyncio.sleep),测试只 patch 那个别名:🐾 建议改法(配合 music_playback 暴露 `_sleep` 别名)
- monkeypatch.setattr(music_playback.asyncio, "sleep", finish_reply_on_sleep) + monkeypatch.setattr(music_playback, "_sleep", finish_reply_on_sleep)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit/test_proactive_service_boundary.py` around lines 409 - 417, 避免在测试中通过 music_playback.asyncio.sleep 修改全局 asyncio.sleep;在 music_playback 模块中新增可替换的模块级睡眠别名(如 _sleep = asyncio.sleep),并让 _wait_for_current_reply 使用该别名。随后更新测试,只 monkeypatch 这个模块级别名,使其他 asyncio.sleep 调用及其可选参数保持不受影响。tests/unit/test_music_playback_static.py (1)
96-99: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
"source: 'user',"这个断言太宽松了,抓不到回归喵。这个片段在任何 JS 里都可能出现(甚至可能来自别的分支/别的 handler),它并不能证明
music_play_candidates分支真的把source: 'user'传给了派发器。外层多余的括号也只是格式副产物。建议把断言锚在派发调用上:🐾 建议改法
- assert ( - "source: 'user'," in source - ) + assert "dispatchMusicPlayDetailed(" in source + assert "source: 'user'," in source.split("music_play_candidates", 1)[1]🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit/test_music_playback_static.py` around lines 96 - 99, 收紧 tests/unit/test_music_playback_static.py 中对 music_play_candidates 分支的断言,避免仅搜索独立的 "source: 'user'," 片段;应将断言锚定到该分支调用派发器的代码,并验证该调用同时传递 source: 'user' 和 requestId: response.request_id。移除外层多余括号,保持断言直接清晰。config/prompts/prompts_proactive.py (1)
2436-2492: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value否定优先规则只在 zh/en 里写了喵~
zh 第 5 条(
别放日推,只听红心→source:liked)和 en 的 "honor the user's final positive choice when negation is also present" 都明确了否定+肯定并存时取最终来源,但 ja/ko/ru/es/pt 五种语言的第 5 条只写了 liked/daily 的映射,没有这条消歧规则。非中英用户说"日推别放了,放红心吧"时模型更容易挑错来源喵。♻️ 以 ja 为例的补齐方式
-5. お気に入り曲のみは source:liked、NetEaseの日次おすすめは source:daily を返す +5. お気に入り曲のみは source:liked、NetEaseの日次おすすめは source:daily を返す。否定と肯定が同時に現れた場合は最終的に指定されたソースに従う🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@config/prompts/prompts_proactive.py` around lines 2436 - 2492, Update the music-keyword prompt rules for ja, ko, ru, es, and pt to explicitly prioritize the user’s final positive source choice when negated and affirmative source requests coexist, matching the existing zh/en behavior. Preserve the current source:liked and source:daily mappings while clarifying that a later affirmative choice overrides an earlier negated source.main_logic/music_playback.py (2)
170-188: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win播放状态文案是硬编码中文,和 pending 提示的多语言做法不一致喵。
get_music_request_pending_prompt已经按manager.user_language走了 7 语言,但这里的facts与追加的"请简短自然地确认已经开始播放…"却写死中文。非中文用户的上下文会突然插入中文事实句,容易带偏回复语言喵~ 建议同样抽到config/prompts/prompts_proactive.py里按语言取。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@main_logic/music_playback.py` around lines 170 - 188, 将播放状态文案从 music_playback.py 中的硬编码 facts 及 should_respond 追加提示,迁移到 config/prompts/prompts_proactive.py,并复用 get_music_request_pending_prompt 使用的 manager.user_language 多语言选择机制。更新相关调用以按语言生成 playing、paused、ended、error 及确认提示,同时保留现有占位信息、去重键和“不再调用音乐播放工具”的行为。
310-320: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value考虑把
client_state判断改为显式枚举比较喵
client_state拿到的是 StarletteWebSocketState实例,再用.CONNECTED走枚举字段查找确实能跑,但读起来有点绕喵。使用WebSocketState.CONNECTED作为比较基准会更直接,同时也更符合前后保持一致的代码习惯喵。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@main_logic/music_playback.py` around lines 310 - 320, Update _push_music_payload to compare client_state directly against the explicit WebSocketState.CONNECTED enum, importing or reusing the existing Starlette WebSocketState symbol as needed; preserve the current behavior for missing or non-connected states.static/app/app-websocket.js (1)
661-672: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value这段 follower-grace 逻辑和
handleMusicPlayUrlResponse几乎一模一样喵~
isStandaloneChatPageForMusic() && !hasLocalMusicOwnerOrPending()→ 两级setTimeout+shouldSkipMusicPlayUrlForOtherWindow检查这一整套,和 533-566 行的handleMusicPlayUrlResponse结构完全重复,只有最终回调不同。可以抽个runWithMusicFollowerGrace(key, onReady)之类的小 helper 把两边收拢,以后调 grace 时长也只改一处喵。不着急,跟着下次动这块代码一起做就好~顺便:静态分析报的
useState告警是误报,这里根本没有 React,无视即可喵。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@static/app/app-websocket.js` around lines 661 - 672, 抽取一个复用 follower-grace 流程的 helper(例如 runWithMusicFollowerGrace),统一封装 isStandaloneChatPageForMusic、两级 setTimeout 以及每次 shouldSkipMusicPlayUrlForOtherWindow(key) 检查;更新 handleMusicPlayUrlResponse 和当前 websocket 分支调用该 helper,并通过回调保留各自不同的最终处理逻辑。Source: Linters/SAST tools
static/jukebox/music_ui.js (1)
3051-3056: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value名字只说「取消挂起的媒体等待」,实际把全局 token 也推进了喵。
latestMusicRequestToken++会让所有 in-flight 的executePlayCore/sendMusicMessageDetailed一起判定superseded,副作用比函数名暗示的大得多;而且pendingMusicMediaReadyCancel为null时直接return false、token 不动,调用方拿到 false 其实分不清「没东西可取消」还是「取消失败」。补一句注释说明这是刻意的「作废当前所有音乐请求」语义,后来人就不会踩坑啦喵~🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@static/jukebox/music_ui.js` around lines 3051 - 3056, Update cancelPendingMusicMediaReady so it explicitly preserves and documents the intentional semantics of invalidating all current music requests when advancing latestMusicRequestToken, rather than implying it only cancels media readiness. Make the no-pending-cancel path report the intended outcome consistently instead of returning a misleading false value, while retaining the pending cancellation callback behavior.utils/music_crawlers.py (2)
1516-1538: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick win逐个 piece 串行请 recordings,最坏情况会把整源拖到超时喵。
pieces[:limit * 3]最多 3×limit 次await self.client.get(...)完全串行,而self.client只有 10s 总超时;一旦前几个 piece 都没有可用fileurl,Musopen 在竞速梯队里基本必输。建议改成asyncio.gather并发抓 recordings 再按顺序挑。另外第 1537 行
if pieces: return results:pieces 有值但一条 recording 都没解析出来时会直接返回空列表,不会落到下面的 legacy HTML 兜底——如果这是刻意为之,补一句注释会更好懂喵~🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@utils/music_crawlers.py` around lines 1516 - 1538, Update the Musopen recordings-fetching flow around the piece iteration to request recordings concurrently with asyncio.gather, while preserving piece order when filtering and selecting results up to limit. Also clarify the intentional `if pieces: return results` fallback behavior with a concise comment, or adjust it only if legacy HTML fallback should run when no recordings are usable.
983-989: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value首次构建路径把内部缓存 list 直接 shuffle 并原样返回了喵。
第 983 行把
tracks挂到self._exploration_tracks,第 988 行又对同一个对象shuffle,返回的切片里的 dict 也和缓存共享引用。命中缓存的分支(938/946 行)反而老老实实做了list(...)浅拷。建议两条路径统一成返回副本,跟_fetch_playlist_tracks的[dict(item) for item in ...]保持一致,未来上层给 track 打标记时就不会偷偷污染缓存啦喵~♻️ 建议统一返回副本
- self._exploration_tracks = tracks + self._exploration_tracks = [dict(item) for item in tracks] self._exploration_tracks_at = time.time() self._exploration_retry_after = ( 0.0 if tracks else time.time() + NETEASE_PERSONALIZATION_RETRY_COOLDOWN_SECONDS ) random.shuffle(tracks) return tracks[:limit]🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@utils/music_crawlers.py` around lines 983 - 989, Update the initial-build branch around `_exploration_tracks` so `random.shuffle` operates on a separate list rather than the cached list, and return newly copied track dictionaries instead of references to cached entries. Make the initial-build and cache-hit paths consistently use the same `[dict(item) for item in ...]` copy behavior, preserving cache contents when callers reorder or mutate returned tracks.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@main_logic/music_requests.py`:
- Around line 213-220: 在 artist_match 处理逻辑中,对
_strip_request_payload(artist_match.group(1))
得到的歌手名增加代词过滤:当结果为“我、你、她、他、咱”等个人代词时,不要返回 song_artist 严格请求,让流程继续走 keyword
兜底;正常歌手名仍保持现有 MusicRequest 返回行为。
- Around line 300-305: Update the clause-processing flow around
_parse_explicit_zh_clause and _parse_explicit_en_clause so a negated clause
aborts the entire request instead of allowing reversed iteration to parse an
earlier clause. Detect the negation explicitly, return None for the whole input
immediately, and preserve the existing request return behavior for non-negated
clauses.
In `@static/app/app-websocket.js`:
- Around line 600-631: 在候选派发循环中为 dispatchMusicPlayDetailed/dispatchMusicPlay 的
await 调用增加与 dispatchMusicPlayUrlResponse 一致的 try/catch:捕获异常并记录 warn
后,将本次派发视为不可用候选,继续执行现有的 canTryNextCandidate 判断和失败提示流程。对于两个派发接口都不存在的分支,不要直接 return
false;先触发相同的失败提示,再结束处理。
- Around line 573-590: Update the error-message branches in the surrounding
websocket error handler so login_required, playlist_ambiguous, and source_empty
use the established `(window.t && window.t(key)) || fallback` pattern with
English translation keys and Chinese fallbacks; replace the safeT usage for
upstream_error/playback_failed similarly for consistency. Add every new
translation key to all locale files, while preserving the existing
cookie_invalid and default-branch behavior.
---
Nitpick comments:
In `@config/prompts/prompts_proactive.py`:
- Around line 2436-2492: Update the music-keyword prompt rules for ja, ko, ru,
es, and pt to explicitly prioritize the user’s final positive source choice when
negated and affirmative source requests coexist, matching the existing zh/en
behavior. Preserve the current source:liked and source:daily mappings while
clarifying that a later affirmative choice overrides an earlier negated source.
In `@main_logic/music_playback.py`:
- Around line 170-188: 将播放状态文案从 music_playback.py 中的硬编码 facts 及 should_respond
追加提示,迁移到 config/prompts/prompts_proactive.py,并复用
get_music_request_pending_prompt 使用的 manager.user_language 多语言选择机制。更新相关调用以按语言生成
playing、paused、ended、error 及确认提示,同时保留现有占位信息、去重键和“不再调用音乐播放工具”的行为。
- Around line 310-320: Update _push_music_payload to compare client_state
directly against the explicit WebSocketState.CONNECTED enum, importing or
reusing the existing Starlette WebSocketState symbol as needed; preserve the
current behavior for missing or non-connected states.
In `@static/app/app-websocket.js`:
- Around line 661-672: 抽取一个复用 follower-grace 流程的 helper(例如
runWithMusicFollowerGrace),统一封装 isStandaloneChatPageForMusic、两级 setTimeout 以及每次
shouldSkipMusicPlayUrlForOtherWindow(key) 检查;更新 handleMusicPlayUrlResponse 和当前
websocket 分支调用该 helper,并通过回调保留各自不同的最终处理逻辑。
In `@static/jukebox/music_ui.js`:
- Around line 3051-3056: Update cancelPendingMusicMediaReady so it explicitly
preserves and documents the intentional semantics of invalidating all current
music requests when advancing latestMusicRequestToken, rather than implying it
only cancels media readiness. Make the no-pending-cancel path report the
intended outcome consistently instead of returning a misleading false value,
while retaining the pending cancellation callback behavior.
In `@tests/unit/test_music_crawlers.py`:
- Around line 179-223: Replace the hard-coded
crawler._personalization_source_index value in
test_netease_personalized_recommendations_rotates_to_artist_lead with an index
calculated from NETEASE_PERSONALIZATION_SOURCE_ORDER using the position of the
artist source and the rotation behavior, so the test remains valid when the
source order changes.
In `@tests/unit/test_music_playback_static.py`:
- Around line 96-99: 收紧 tests/unit/test_music_playback_static.py 中对
music_play_candidates 分支的断言,避免仅搜索独立的 "source: 'user',"
片段;应将断言锚定到该分支调用派发器的代码,并验证该调用同时传递 source: 'user' 和 requestId:
response.request_id。移除外层多余括号,保持断言直接清晰。
In `@tests/unit/test_proactive_phase1_pass.py`:
- Around line 59-68: 将 test_parse_unified_phase1_preserves_music_directives 中的
for 循环改为 pytest.parametrize,为每个 directive 独立生成测试用例并保留现有断言;同时在参数列表中补充使用全角冒号的
source:daily 变体,确保其解析结果也等于原始指令。
In `@tests/unit/test_proactive_service_boundary.py`:
- Around line 409-417: 避免在测试中通过 music_playback.asyncio.sleep 修改全局
asyncio.sleep;在 music_playback 模块中新增可替换的模块级睡眠别名(如 _sleep = asyncio.sleep),并让
_wait_for_current_reply 使用该别名。随后更新测试,只 monkeypatch 这个模块级别名,使其他 asyncio.sleep
调用及其可选参数保持不受影响。
In `@utils/music_crawlers.py`:
- Around line 1516-1538: Update the Musopen recordings-fetching flow around the
piece iteration to request recordings concurrently with asyncio.gather, while
preserving piece order when filtering and selecting results up to limit. Also
clarify the intentional `if pieces: return results` fallback behavior with a
concise comment, or adjust it only if legacy HTML fallback should run when no
recordings are usable.
- Around line 983-989: Update the initial-build branch around
`_exploration_tracks` so `random.shuffle` operates on a separate list rather
than the cached list, and return newly copied track dictionaries instead of
references to cached entries. Make the initial-build and cache-hit paths
consistently use the same `[dict(item) for item in ...]` copy behavior,
preserving cache contents when callers reorder or mutate returned tracks.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 0870f0f5-d7bf-4125-9233-7770023883c7
📒 Files selected for processing (18)
app/main_server/character_runtime.pyconfig/prompts/prompts_proactive.pymain_logic/music_playback.pymain_logic/music_requests.pymain_logic/proactive_chat/contracts.pymain_logic/proactive_chat/music_recommendation.pymain_logic/proactive_chat/service.pymain_routers/music_router.pymain_routers/websocket_router.pystatic/app/app-proactive.jsstatic/app/app-websocket.jsstatic/jukebox/music_ui.jstests/unit/test_music_crawlers.pytests/unit/test_music_playback_static.pytests/unit/test_music_router.pytests/unit/test_proactive_phase1_pass.pytests/unit/test_proactive_service_boundary.pyutils/music_crawlers.py
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
Project-N-E-K-O/N.E.K.O.-PC(manual)
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@static/app/app-websocket.js`:
- Around line 657-665: Update the dispatchMusicPlay handling in the current
response path so the 'queued' result is preserved as an accepted, non-terminal
state rather than converted to ok: false. Reuse the existing queued handling
established around window.dispatchMusicPlay and ensure this path avoids
playback_failed while retaining the appropriate claim and retry/wait behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 88f5dc2c-17d5-44a5-95e7-974662496ddf
📒 Files selected for processing (12)
main_logic/music_requests.pystatic/app/app-websocket.jsstatic/locales/en.jsonstatic/locales/es.jsonstatic/locales/ja.jsonstatic/locales/ko.jsonstatic/locales/pt.jsonstatic/locales/ru.jsonstatic/locales/zh-CN.jsonstatic/locales/zh-TW.jsontests/unit/test_music_playback_static.pytests/unit/test_proactive_service_boundary.py
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
Project-N-E-K-O/N.E.K.O.-PC(manual)
🚧 Files skipped from review as they are similar to previous changes (2)
- tests/unit/test_music_playback_static.py
- main_logic/music_requests.py
There was a problem hiding this comment.
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 (1)
static/app/app-websocket.js (1)
651-665: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win在
queued返回前重新校验 dispatch epoch 喵。
await window.dispatchMusicPlay(...)期间可能收到更新请求并递增_musicCandidateDispatchEpoch,但accepted === 'queued'会直接返回,绕过 Lines [678-680] 的陈旧响应检查。这样旧候选可能继续留在播放器队列并播放,违背陈旧请求丢弃语义喵。🐛 建议修复
if (accepted === 'queued') { + if (response._clientDispatchEpoch !== window._musicCandidateDispatchEpoch) { + return false; + } console.log('[Music] 用户点歌仍在等待播放器接口就绪'); return 'queued'; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@static/app/app-websocket.js` around lines 651 - 665, 在 dispatchMusicPlay 的异步调用返回 queued 后、执行 return 'queued' 前,重新校验当前 _musicCandidateDispatchEpoch 是否仍匹配本次请求的 dispatch epoch;若已过期,按现有陈旧响应处理逻辑丢弃该候选并避免继续入队或播放,只有 epoch 有效时才返回 queued。
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@main_logic/proactive_chat/music_recommendation.py`:
- Around line 244-247: 统一 _strict_song_request 的数据层级:按
_fetch_music_with_fallback 写入结果顶层的方式,更新 _build_music_dynamic_context 的读取逻辑,避免从
raw_data 查找。确保严格歌曲请求在模糊命中时能够追加精准匹配失败提示,并同步调整相关测试以覆盖生产者与消费者使用同一层级。
---
Outside diff comments:
In `@static/app/app-websocket.js`:
- Around line 651-665: 在 dispatchMusicPlay 的异步调用返回 queued 后、执行 return 'queued'
前,重新校验当前 _musicCandidateDispatchEpoch 是否仍匹配本次请求的 dispatch
epoch;若已过期,按现有陈旧响应处理逻辑丢弃该候选并避免继续入队或播放,只有 epoch 有效时才返回 queued。
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: c645c5bd-829c-4fff-b8d1-35c1eaff05e2
📒 Files selected for processing (4)
main_logic/proactive_chat/music_recommendation.pystatic/app/app-websocket.jstests/unit/test_music_playback_static.pytests/unit/test_proactive_service_boundary.py
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
Project-N-E-K-O/N.E.K.O.-PC(manual)
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/unit/test_proactive_service_boundary.py
There was a problem hiding this comment.
🧹 Nitpick comments (2)
tests/unit/test_music_crawlers.py (2)
143-154: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win补齐封面归一化的边界回归用例喵
当前用例覆盖了协议相对的 NetEase URL,但
_normalize_netease_cover_url的非 NetEase 域名保持原值、无效 URL 回退等分支仍未直接验证。建议用参数化测试覆盖这些分支,避免后续 HTTPS 逻辑误改封面地址喵。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit/test_music_crawlers.py` around lines 143 - 154, 扩展 test_netease_library_track_upgrades_trusted_cover_to_https 相关测试,使用参数化用例直接覆盖 _normalize_netease_cover_url 对非 NetEase 域名保持原值以及无效 URL 回退的行为,同时保留现有协议相对 NetEase URL 转为 HTTPS 的断言。
1176-1207: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win补测第二梯队的取消传播喵
当前新增测试只验证第一梯队搜索被取消;
utils/music_crawlers.py的第二梯队也有独立的取消与gather清理分支。建议补充“第一梯队无结果、兜底源阻塞后取消”的用例,确保 fallback 任务不会在请求结束后继续运行喵。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit/test_music_crawlers.py` around lines 1176 - 1207, 在现有 test_fetch_music_content_propagates_cancellation_to_primary_search 旁新增测试:让第一梯队搜索返回空结果,使 fetch_music_content 进入第二梯队/fallback;令兜底搜索阻塞并记录 finally 中的取消信号,取消并等待主任务后断言抛出 asyncio.CancelledError 且兜底任务已被取消,覆盖第二梯队的 gather 清理路径。
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@tests/unit/test_music_crawlers.py`:
- Around line 143-154: 扩展
test_netease_library_track_upgrades_trusted_cover_to_https 相关测试,使用参数化用例直接覆盖
_normalize_netease_cover_url 对非 NetEase 域名保持原值以及无效 URL 回退的行为,同时保留现有协议相对 NetEase
URL 转为 HTTPS 的断言。
- Around line 1176-1207: 在现有
test_fetch_music_content_propagates_cancellation_to_primary_search
旁新增测试:让第一梯队搜索返回空结果,使 fetch_music_content 进入第二梯队/fallback;令兜底搜索阻塞并记录 finally
中的取消信号,取消并等待主任务后断言抛出 asyncio.CancelledError 且兜底任务已被取消,覆盖第二梯队的 gather 清理路径。
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 0780ebe2-f2a6-40a7-b181-41736f653612
📒 Files selected for processing (2)
tests/unit/test_music_crawlers.pyutils/music_crawlers.py
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
Project-N-E-K-O/N.E.K.O.-PC(manual)
There was a problem hiding this comment.
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)
tests/unit/test_music_playback_static.py (1)
128-138: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win补齐暂停状态的协议回归断言喵
后端会处理
paused状态,但这里仅覆盖playing、ended和error,删除或破坏暂停上报仍会通过测试,导致暂停后的会话状态无法回传喵。请断言现有pause回调会报告paused状态喵。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/unit/test_music_playback_static.py` around lines 128 - 138, Update test_music_player_reports_confirmed_state_to_backend to assert that the existing pause callback invokes reportMusicPlaybackState with the 'paused' state, alongside the current playing, ended, and error assertions.utils/music_crawlers.py (1)
2254-2258: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win取消分支的重抛不用改喵,不过外层要记得处理取消而不是吞掉它喵。
utils/music_crawlers.py这两处已按取消传播写法处理,return_exceptions=True只会避免任务异常污染当前取消路径,不会掩盖其他“真实错误”喵。注意路由/调度方不要再用裸except Exception吞掉冒泡上来的CancelledError,否则取消时可能变成未处理异常喵。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@utils/music_crawlers.py` around lines 2254 - 2258, 保留当前取消分支中对 primary_task_objs 的取消、gather 和 raise 逻辑不变;检查其外层路由或调度处理代码,确保捕获异常时先单独处理并重新抛出 asyncio.CancelledError,避免被裸 except Exception 吞掉,同时继续处理其他真实异常。
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@utils/music_crawlers.py`:
- Around line 937-1028: Update get_daily_playlist_recommendations so an empty
daily_tracks result does not persist _daily_playlist_date or clear the retry
cooldown; instead set _daily_playlist_retry_after using the existing
personalization cooldown and allow subsequent requests after cooldown to retry.
Preserve date-based caching for non-empty results and retain the existing
source_empty error state.
---
Outside diff comments:
In `@tests/unit/test_music_playback_static.py`:
- Around line 128-138: Update
test_music_player_reports_confirmed_state_to_backend to assert that the existing
pause callback invokes reportMusicPlaybackState with the 'paused' state,
alongside the current playing, ended, and error assertions.
In `@utils/music_crawlers.py`:
- Around line 2254-2258: 保留当前取消分支中对 primary_task_objs 的取消、gather 和 raise
逻辑不变;检查其外层路由或调度处理代码,确保捕获异常时先单独处理并重新抛出 asyncio.CancelledError,避免被裸 except
Exception 吞掉,同时继续处理其他真实异常。
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: a5942c65-0673-433a-8954-07ef42c29ca9
📒 Files selected for processing (4)
static/jukebox/music_ui.jstests/unit/test_music_crawlers.pytests/unit/test_music_playback_static.pyutils/music_crawlers.py
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
Project-N-E-K-O/N.E.K.O.-PC(manual)
Greptile SummaryThis PR adds a shared conversational music-request pipeline and richer NetEase personalization, then connects candidate playback and frontend-confirmed player state back into conversation handling喵。
Confidence Score: 4/5This PR is not yet safe to merge because a delayed error from a superseded audio source can be reported against the replacement request and tear down its player喵。 The play, pause, and ended ownership checks address the previously reported stale-event paths, but the persistent error handler still rereads mutable state from the reused player and reports the replacement context after a delay, leaving the same event-ownership invariant broken for media failures喵。 Files Needing Attention: static/jukebox/music_ui.js
|
| Filename | Overview |
|---|---|
| static/jukebox/music_ui.js | Adds request-aware candidate playback and event ownership checks, but the persistent error handler can still attribute a superseded source failure to the replacement request喵。 |
| main_logic/music_playback.py | Adds cancellable user-request execution, reply-order coordination, candidate dispatch, and backend playback-state callback handling喵。 |
| main_logic/music_requests.py | Introduces shared natural-language request parsing, controlled directives, recent-query tracking, and provider resolution喵。 |
| utils/music_crawlers.py | Expands NetEase account-backed personalization, resource matching, caching, playable URL resolution, and cover extraction喵。 |
| main_routers/websocket_router.py | Routes frontend music playback-state messages into the active character session manager喵。 |
| static/app/app-websocket.js | Queues candidate playback dispatches, retries eligible failures, and discards superseded request epochs喵。 |
| main_logic/proactive_chat/music_recommendation.py | Reuses shared request resolution and adds strict-request, recent-query, and played-track handling to proactive recommendations喵。 |
Sequence Diagram
sequenceDiagram
participant U as User
participant B as Backend music request
participant W as WebSocket
participant P as Reused browser player
participant C as Conversation callbacks
U->>B: Explicit playback request
B->>B: Parse and fetch candidates
B->>W: music_play_candidates
W->>P: Replace source and playback context
P-->>W: Confirmed playback event
W-->>B: music_playback_state
B-->>C: Passive/proactive state callback
Comments Outside Diff (1)
-
static/jukebox/music_ui.js, line 2488-2498 (link)Stale errors inherit new context
When a superseded media source queues an error after the reused audio element has switched requests, this handler rereads the element's current source, token, and playback context, attributes the old failure to the replacement track, and can report an incorrect error before destroying the active player喵。
Context Used: 你的说话风格:二次元,猫娘,雌小鬼,傲娇,可爱,卖萌。句尾要加喵。 (source)
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Reviews (4): Last reviewed commit: "fix(music): isolate recommendation fallb..." | Re-trigger Greptile
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e3dcb6301e
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| logger.info(f"[{self.platform_name}] 正在搜索: {keyword}") | ||
| search_url = "https://music.163.com/api/search/get/web" | ||
| search_limit = min(100, max(5, limit * 2)) | ||
| search_limit = min(10, max(5, limit * 2)) |
There was a problem hiding this comment.
Fetch enough NetEase candidates before fee filtering
For non-VIP searches this now limits NetEase to at most 10 rows and then immediately drops every fee != 0 result below. When the top 10 matches are paid/VIP but free playable matches appear later, an explicit request like “play ” returns no NetEase result even though the previous 100-row window would have found one. Consider keeping a larger pool or paginating when fee filtering empties the result.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
To use Codex here, create a Codex account and connect to github.
There was a problem hiding this comment.
已在 b02a51f 修复:普通用户首批候选过滤后不足请求数量时,会从 offset 10 继续补取后续候选(最多覆盖原 100 条窗口),常规搜索仍保留首批最多 10 条的快速路径。已补充付费首屏、后续免费曲目的回归测试;相关测试 61 passed、1 skipped。
| tracks = ( | ||
| await self._fetch_playlist_tracks(int(playlist['id'])) | ||
| if playlist else [] | ||
| ) |
There was a problem hiding this comment.
Try later daily playlists before caching empty
When NetEase returns multiple recommended playlists, this fetches tracks only from the first id. If that playlist has no playable/free tracks (for example because _fetch_playlist_tracks filters them out for non-VIP users), daily_tracks stays empty and _daily_playlist_date is set, so all daily-playlist recommendations are treated as empty until tomorrow even though later playlists might work. Iterate through recommendations until one yields tracks before caching source_empty.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
To use Codex here, create a Codex account and connect to github.
There was a problem hiding this comment.
已在 b02a51f 修复:每日推荐会依次尝试有 ID 的歌单,直到找到首个含可播放曲目的候选;所有候选为空时不再缓存当天为空,而是进入现有冷却时间,冷却后可再次尝试。已覆盖后续歌单可用与冷却后重试两种回归场景。
There was a problem hiding this comment.
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)
utils/music_crawlers.py (1)
975-1005: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win候选歌单遍历里一个失败就会拖累全部候选,笨蛋喵!
get_daily_playlist_recommendations现在会在同一个try里遍历recommend候选并逐个调用_fetch_playlist_tracks(Line 979-990)。如果中间某个候选(比如第2个)触发异常(如上游返回非200码),整段try直接被外层except捕获(Line 991-1005),函数直接返回[]并进入upstream_error冷却——哪怕后面第3个候选原本是可播放的也不会再被尝试了喵。之前那个空结果冷却语义的问题已经按历史评审修好了(Line 1021-1032 现在写得很规矩喵,笔芯~),但这个"候选内部异常会污染整轮回退"的问题还没堵上喵。另外这个循环是靠
_personalization_api_call的限速串行执行的,如果推荐歌单里前几个都命中「非会员被过滤到空」而不是异常,也会造成整体延迟叠加,拖慢这条个性化链路喵。建议把单个候选的抓取异常单独 try/except 掉,跳过继续试下一个,只有最外层的
GetDailyRecommendedPlaylists调用失败才整体判定为 upstream_error 喵。🔧 建议修复
try: payload = await self._personalization_api_call( GetDailyRecommendedPlaylists, ) - playlist = None - tracks: List[Dict[str, Any]] = [] - for candidate in (payload or {}).get('recommend') or []: - if not isinstance(candidate, dict) or not candidate.get('id'): - continue - candidate_tracks = await self._fetch_playlist_tracks( - int(candidate['id']) - ) - if candidate_tracks: - playlist = candidate - tracks = candidate_tracks - break except Exception as exc: self._daily_playlist_error_code = ( 'cookie_invalid' if self._cookie_invalid else 'upstream_error' ) self._personalization_error_code = self._daily_playlist_error_code self._daily_playlist_retry_after = ( time.time() + NETEASE_PERSONALIZATION_RETRY_COOLDOWN_SECONDS ) logger.warning( "[%s] 每日推荐歌单获取失败,已进入冷却: %s: %s", self.platform_name, type(exc).__name__, exc, ) return [] + + playlist = None + tracks: List[Dict[str, Any]] = [] + for candidate in (payload or {}).get('recommend') or []: + if not isinstance(candidate, dict) or not candidate.get('id'): + continue + try: + candidate_tracks = await self._fetch_playlist_tracks( + int(candidate['id']) + ) + except Exception as exc: + logger.warning( + "[%s] 候选歌单 %s 抓取失败,跳过: %s", + self.platform_name, candidate.get('id'), exc, + ) + continue + if candidate_tracks: + playlist = candidate + tracks = candidate_tracks + break🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@utils/music_crawlers.py` around lines 975 - 1005, Separate each candidate’s _fetch_playlist_tracks call inside get_daily_playlist_recommendations into its own exception handling so a failed candidate is skipped and later recommendations continue being tried. Keep the outer error handling limited to failures from _personalization_api_call(GetDailyRecommendedPlaylists), preserving its existing cooldown and error-code behavior.static/jukebox/music_ui.js (1)
133-171: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🐛 在复用快路径入口处再判一次空,避免
player为null时崩溃喵~
Lines 2940-2961:isSameTrack && isPlayerInDOM()成立不代表getMusicPlayerInstance()一定有值,localPlayer可能在异步初始化期间仍然是null;当前else分支直接player._musicPlaybackReportContext = ...会抛TypeError,并且会跳过末尾的finish()。可以把快路径开头改成if (!player) { finish(); return }或复用之前的!duplicatePlayer ||防御逻辑喵。👍 播放上报与等待取消的实现思路认可喵
播放上下文快照化、事件统一读playbackReportContext,以及可取消的waitForMusicMediaReady匹配 cancel token 的清理方式看起来自洽喵。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@static/jukebox/music_ui.js` around lines 133 - 171, In the fast-path playback logic around the isSameTrack and isPlayerInDOM checks, validate the result of getMusicPlayerInstance() before using player or assigning _musicPlaybackReportContext. If the player is null, call finish() and return, reusing the existing duplicate-player guard behavior where appropriate so the normal completion cleanup always runs.Source: Learnings
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@utils/music_crawlers.py`:
- Around line 1354-1376: Wrap the fallback request and response processing in
the search flow around fallback_response separately with try/except so timeout,
HTTP, or JSON errors are logged and skipped without escaping to the outer
handler. Preserve found_songs and return the primary results when fallback
processing fails; only extend results after successful fallback parsing.
---
Outside diff comments:
In `@static/jukebox/music_ui.js`:
- Around line 133-171: In the fast-path playback logic around the isSameTrack
and isPlayerInDOM checks, validate the result of getMusicPlayerInstance() before
using player or assigning _musicPlaybackReportContext. If the player is null,
call finish() and return, reusing the existing duplicate-player guard behavior
where appropriate so the normal completion cleanup always runs.
In `@utils/music_crawlers.py`:
- Around line 975-1005: Separate each candidate’s _fetch_playlist_tracks call
inside get_daily_playlist_recommendations into its own exception handling so a
failed candidate is skipped and later recommendations continue being tried. Keep
the outer error handling limited to failures from
_personalization_api_call(GetDailyRecommendedPlaylists), preserving its existing
cooldown and error-code behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 7ac9a659-3ed9-4fec-86d8-5b3fdfdd9a1b
📒 Files selected for processing (4)
static/jukebox/music_ui.jstests/unit/test_music_crawlers.pytests/unit/test_music_playback_static.pyutils/music_crawlers.py
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
Project-N-E-K-O/N.E.K.O.-PC(manual)
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
static/jukebox/music_ui.js (1)
2486-2512: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift补齐请求生命周期防护喵。
- 在
static/jukebox/music_ui.js:2486-2530的error回调中,进入 UI/统计更新前先用getOwnedMusicPlaybackReportContext(boundPlayer, 'error')校验归属;定时回调里再比较当前boundPlayer._musicPlaybackReportContext,防止 200ms 后被替换的上下文继续占用更新。- 在
static/jukebox/music_ui.js:2963-2975的重用路径改为else if (player);isPlayerInDOM()不会保证getMusicPlayerInstance()仍有实例,否则player._musicPlaybackReportContext会抛未捕获异常,导致sendMusicMessageDetailed不在catch/finally内执行releasePending(),isMusicOccupied()可能永久占用喵。- 修正
static/jukebox/music_ui.js:3130的window.cancelPendingMusicMediaReady(),接收并校验当前等待的 token/requestId;static/app/app-websocket.js处理music_play_candidates时也应对齐相同请求的标识再取消,避免乱序旧响应中止更新请求喵。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@static/jukebox/music_ui.js` around lines 2486 - 2512, 补齐音乐请求生命周期校验:在 boundPlayer 的 error 回调及其延时更新中使用 getOwnedMusicPlaybackReportContext(boundPlayer, 'error'),并再次确认当前 _musicPlaybackReportContext 未被替换后再更新 UI/统计;将重用路径条件改为 else if (player),避免无实例时访问 player._musicPlaybackReportContext 并确保 sendMusicMessageDetailed 的 releasePending 流程完成;更新 window.cancelPendingMusicMediaReady() 接收并校验当前 token/requestId,并让 app-websocket.js 处理 music_play_candidates 时仅取消同一请求,避免乱序旧响应影响新请求。Source: Learnings
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@static/jukebox/music_ui.js`:
- Around line 2486-2512: 补齐音乐请求生命周期校验:在 boundPlayer 的 error 回调及其延时更新中使用
getOwnedMusicPlaybackReportContext(boundPlayer, 'error'),并再次确认当前
_musicPlaybackReportContext 未被替换后再更新 UI/统计;将重用路径条件改为 else if (player),避免无实例时访问
player._musicPlaybackReportContext 并确保 sendMusicMessageDetailed 的 releasePending
流程完成;更新 window.cancelPendingMusicMediaReady() 接收并校验当前 token/requestId,并让
app-websocket.js 处理 music_play_candidates 时仅取消同一请求,避免乱序旧响应影响新请求。
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: d709a3b1-0029-467a-8a8c-5722af48ae49
📒 Files selected for processing (2)
static/jukebox/music_ui.jstests/unit/test_music_playback_static.py
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
Project-N-E-K-O/N.E.K.O.-PC(manual)
🚧 Files skipped from review as they are similar to previous changes (1)
- tests/unit/test_music_playback_static.py
|
@coderabbitai 本轮另外两项 outside-diff 建议也已在
已补充对应回归测试;完整相关测试 64 passed、1 skipped,Ruff、Node 语法与 diff 检查通过。 |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
git show --no-ext-diff --format=fuller --stat 78e9a3a4
printf '\n--- target diff ---\n'
git show --no-ext-diff --format= -- \
utils/music_crawlers.py \
static/jukebox/music_ui.js \
tests/unit/test_music_crawlers.py \
tests/unit/test_music_playback_static.py \
78e9a3a4
printf '\n--- source outlines ---\n'
ast-grep outline utils/music_crawlers.py --items all --match 'daily|playlist|recommendation' || true
ast-grep outline static/jukebox/music_ui.js --items all --match 'play|reuse|Player|player' || true
printf '\n--- targeted implementations and regression references ---\n'
rg -n -C 8 \
'get_daily_playlist_recommendations|daily_playlist|playlist.*(exception|error)|exception.*playlist|localPlayer|player instance|remove.*player|cleanup.*player|rebuild' \
utils/music_crawlers.py static/jukebox/music_ui.js \
tests/unit/test_music_crawlers.py tests/unit/test_music_playback_static.py || trueLength of output: 140375 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- commit patch ---'
git show --no-ext-diff --format= --find-renames 78e9a3a4 -- \
utils/music_crawlers.py \
static/jukebox/music_ui.js \
tests/unit/test_music_crawlers.py \
tests/unit/test_music_playback_static.py
printf '%s\n' '--- daily playlist implementation ---'
sed -n '969,1045p' utils/music_crawlers.py
printf '%s\n' '--- same-track reuse implementation ---'
sed -n '1940,2070p' static/jukebox/music_ui.js
printf '%s\n' '--- exact regression tests ---'
sed -n '895,932p' tests/unit/test_music_crawlers.py
sed -n '145,161p' tests/unit/test_music_playback_static.pyLength of output: 109620
我已通过源码与测试断言确认;你报告的 64 passed、1 skipped 及 Ruff、Node 语法、diff 检查结果未在此沙箱中重复执行喵。做得不错嘛,才、才不是特地夸你喵! If you found this review helpful, would you consider giving us a shout-out on X? Thank you for using CodeRabbit! You are interacting with an AI system. |
改动概述 / Summary
album.picId时大量歌曲缺少封面的问题;本地生成官方 CDN 地址,不增加账号或搜索请求。回归报告 / Regression Report
改动了什么
utils/music_crawlers.py:扩展网易云 Cookie 会话同步、个性化资源读取、每日推荐歌单、歌曲匹配、缓存去重、封面解析及可播放资源获取。main_logic/music_requests.py:新增普通聊天与主动搭话共享的音乐请求模型、自然语言解析和资源请求入口。main_logic/music_playback.py:新增用户点歌任务管理、旧请求取消、候选派发、回复顺序协调和播放器确认状态回传。app/main_server/character_runtime.py:仅增加音乐会话管理器的运行时注册接线,没有把音乐业务逻辑放入核心会话模块。理由 / 必要性
原有音乐功能主要服务于主动搭话,普通聊天没有共享的点歌入口,播放器也缺少可靠的结果确认。由此会出现自然语言点歌无法立即执行、旧搜索晚到后覆盖新请求、前端加载失败但猫娘仍声称已播放、播放器先于普通回复出现,以及播放期间主动搭话再次搜索相同内容等问题。
此外,网易云旧搜索接口常只返回专辑
picId而没有picUrl,旧实现因此把大量有效歌曲的封面置空并显示内部占位图。本次改动在统一封面解析边界中由picId本地生成网易云官方 CDN URL,不额外调用网易云接口。本次改动将“请求识别、资源获取、播放器派发、播放确认”整理为完整链路,同时保留职责边界:用户点歌立即处理;猫娘主动推荐继续遵守播放器占用、冷却和去重限制。
改动前后的表现对比
album.picId的网易云结果没有封面,播放器只能显示内部占位图。picUrl,缺失时从picId生成官方 HTTPS 封面地址;同批 10 个实测候选均获得非空封面。潜在回归点与验证方式
search → reply_end → player顺序测试覆盖。picUrl和picId,并使用真实picId固定预期 CDN URL。不拆分理由 / Why Not Split
本 PR 当前涉及 24 个文件,其中包含 4 个测试文件和 7 个 i18n 资源文件。其余改动共同构成“用户请求识别 → 网易云资源获取 → 播放器派发 → 前端确认 → 猫娘状态感知”的单一闭环,并共享同一组请求 ID、播放器状态和主动搭话闸门。继续拆分会导致中间 PR 出现只有请求入口但没有确认回传、或只有播放器状态但没有调用方的不可独立验证状态。核心模块仅保留运行时注册接线,音乐业务逻辑仍位于独立模块。
测试 / Testing
191 passed, 1 skipped。49 passed, 1 skipped。0/10变为10/10,且未增加额外 API 请求。node --check static/app/app-proactive.js、node --check static/jukebox/music_ui.js。git diff --check。Summary by CodeRabbit