Phase 4A:建立受控 ASR consumer routing 与语音设置 - #2396
Conversation
WalkthroughChanges该 PR 引入 语音输入路由重构
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related issues
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 |
Greptile SummaryThis PR adds controlled ASR transcript routing and shared voice-recognition settings.
Confidence Score: 5/5This looks safe to merge.
|
| Filename | Overview |
|---|---|
| main_logic/core/asr_runtime.py | Adds resource-optimization handshake handling and token-bound routing into the controlled voice-input path. |
| main_logic/voice_input/registry.py | Adds consumer registration, active-route selection, and token-scoped partial, final, and cancellation dispatch. |
| main_logic/voice_input/consumers/core_chat.py | Adds exact-turn Core preview, final delivery, and cancellation cleanup. |
| main_logic/voice_input/consumers/game.py | Adds game final-transcript routing while keeping unavailable routes closed. |
| static/app/app-audio-capture.js | Adds recognition-settings popover rendering, pending-state display, and cleanup lifecycle handling. |
| static/app/app-settings.js | Adds authoritative resource-optimization state and cross-window voice-settings synchronization. |
Sequence Diagram
sequenceDiagram
participant UI as Voice settings UI
participant WS as WebSocket router
participant ASR as ASR runtime
participant Registry as Voice input registry
participant Consumer as Core or game consumer
UI->>WS: Start session with settings handshake
WS->>ASR: Apply handshake and start session
ASR->>Registry: Begin token-bound utterance
ASR->>Registry: Dispatch partial or final transcript
Registry->>Registry: Validate active token and route
Registry->>Consumer: Deliver authorized event
Consumer-->>ASR: Complete or cancel exact turn
Reviews (14): Last reviewed commit: "fix(voice): preserve session-scoped sett..." | Re-trigger Greptile
c24fdf7 to
99b67c3
Compare
fc68230 to
9730015
Compare
|
@codex 全面审查一下 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 99b67c358f
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
已处理 Codex P1 review(
PR 描述中的对应路由说明也已同步修正。 |
139369a to
aa13f85
Compare
aa13f85 to
fa31299
Compare
|
本次已按 #2345 的最终架构重新收敛并更新堆叠基线:Phase 4A 的 ASR 运行时改动现位于 |
|
两个 P2 已在
边界保持不变:只修改 验证结果:
堆叠检查:#2398、#2408 均未单独修改该前端文件,不应复制同一修复;后续只需按 #2396 → #2398 → #2408 顺序继承新基线。 |
14fe8f5 to
0fb3c77
Compare
Stacked rebase / Provider recovery owner 边界本 PR 已纯 rebase 到修复后的 #2345
精确组合验证:129 passed;Core package contract、Ruff 与 diff check 通过。前端浏览器测试 2 项通过,剩余 1 项仅因本地未生成 React chat bundle 而出现既有静态资源 404,与设置或 Soniox recovery 无关。 |
2d6a6be to
392e89e
Compare
ASR consumer routing 最终边界补充本 PR 的最终职责已收敛为:在最新版 #2345 之上建立统一、受控的 ASR consumer routing;#2345 不再扩容。
验证结果:consumer routing/core 单测 97 passed, |
Restack 完成回执已按上面的收敛边界完成实现整理和依次 restack:
GitHub 上的 stacked base 镜像分支已经同步。目前三层 PR 均为 Draft / CLEAN,祖先关系为: 边界核对:
验证结果:
|
392e89e to
9a58231
Compare
|
@codex review |
|
Codex Review: Didn't find any major issues. Nice work! Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
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". |
d6a479b to
4c6d71d
Compare
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (4)
scripts/check_core_contracts.py (1)
512-529: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value顺手把 docstring 里的参数名也改成复数形态吧喵~
签名已经变成
str | tuple[str, ...],但文档里还写着单数的forbidden_prefix,读起来会以为只能传一个前缀喵。📝 建议改动
- Flags a literal target inside ``forbidden_prefix`` the same way the static - import ban does, and any non-literal target outright — the gate cannot - prove a computed module name stays on the right side of the boundary. + ``forbidden_prefix`` accepts one prefix or a tuple of them. Flags a literal + target inside any of them the same way the static import ban does, and any + non-literal target outright — the gate cannot prove a computed module name + stays on the right side of the boundary.🤖 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 `@scripts/check_core_contracts.py` around lines 512 - 529, Update the _dynamic_import_violations docstring to refer to the plural forbidden_prefixes parameter concept consistently, reflecting that the function accepts multiple forbidden prefixes while leaving the implementation unchanged.tests/unit/test_check_core_contracts.py (1)
337-368: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win参数化里少了一个「非白名单顶层包」的用例喵。
现在覆盖了
main_logic / main_routers / utils三个受管根,但scripts/check_core_contracts.py的guarded_roots只有这三个,所以像import plugin.plugins.xxx这样的仓内反向依赖是不会被拦的喵。加一条会红的用例,才能把那个缺口钉住喵~💚 建议补充用例
"import main_routers.game_router\n", "from utils import preferences\n", + "import plugin.plugins.demo\n", "import importlib\nimportlib.import_module('main_logic.core')\n",(需要先按
scripts/check_core_contracts.py那条评论扩大guarded_roots,否则这条用例会失败喵。)🤖 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_check_core_contracts.py` around lines 337 - 368, 补充 test_run_flags_forbidden_voice_input_dependencies 的参数化输入,覆盖非白名单顶层包 plugin 的导入场景,并确认仍产生 VOICE_INPUT_LAYERING 违规。同步扩大 scripts/check_core_contracts.py 中 guarded_roots 的检查范围,使该反向依赖能够被 contract_checker.run 检测到;保留现有 main_logic、main_routers 和 utils 用例行为不变。static/app/app-settings.js (1)
1290-1312: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win优化开关也加上 ASR 的
writerId裁决吧喵~
optimizationWriteIsNewer在meta.writeId === _lastAppliedSharedWriteId时只比较显式标记,而 ASR 在平局时会再用writerId打破顺序、保证并发同 id 写入双方选出同一赢家。这个优化钥匙也共享到每次 snapshot,如果直接复用 ASR 那套平局比较,能避免本窗口 own-id 的显式选择被同秒并发写入盖掉喵。🤖 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-settings.js` around lines 1290 - 1312, Update optimizationWriteIsNewer for voiceInputResourceOptimizationEnabled to use the same writerId tie-breaker as the independentAsrEnabled arbitration logic when writeId values are equal. Preserve the explicit-change requirement while selecting a deterministic winner for concurrent same-id writes, so the current window’s own writer is not overwritten by another writer.main_logic/core/asr_runtime.py (1)
2136-2140: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win丢弃的 final 建议留一条日志喵~
dispatch_final返回REJECTED时代表一整句用户语音被 fail-closed 丢掉了(token 过期、consumer 不可用、重复 final),但这里把返回值直接扔掉,日志里什么都看不到。线上排查"我说了话但没反应"会很痛苦喵。♻️ 建议加一条 debug 级观测
async def _dispatch_voice_input_final( self, event: VoiceTranscriptEvent, ) -> None: - await self._voice_input_registry.dispatch_final(event) + result = await self._voice_input_registry.dispatch_final(event) + if result is VoiceInputDispatchResult.REJECTED: + logger.debug( + "[%s] voice input final rejected turn=%s-%s", + self.lanlan_name, + event.turn_token.ingress.session_epoch, + event.turn_token.turn_id, + )(需要从
main_logic.voice_input补VoiceInputDispatchResult导入喵。)🤖 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/core/asr_runtime.py` around lines 2136 - 2140, Update _dispatch_voice_input_final to capture the result from _voice_input_registry.dispatch_final(event), import VoiceInputDispatchResult from main_logic.voice_input, and emit a debug log when the result is REJECTED so discarded final transcripts are observable.
🤖 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/core/asr_runtime.py`:
- Around line 748-751: 调整 _abort_independent_asr 的执行顺序,先调用
self._asr_runtime.abort(reason) 完成 generation bump,再执行
_invalidate_voice_pcm_sync(reason) 和 _voice_input_registry.wait_idle();确保
backpressure 路径在等待取消任务前就拒绝后续过时帧,并保留现有清理流程。
- Around line 2263-2272: 将 Core 语音 turn 处理流程中 `token`、`_voice_lease_owner` 和
`session_ref` 的三个前置校验移入覆盖后续逻辑的 `try/finally`,确保任何提前 return 都会执行现有的
`_abandon_core_voice_turn(...)` 清理。保持校验条件和成功路径不变,仅扩大 `finally` 的保护范围以避免
external-turn pause 泄漏。
In `@scripts/check_core_contracts.py`:
- Around line 1104-1126: 扩大 voice_input 导入边界检查中 guarded_roots
的覆盖范围,使仓库内非标准库/第三方包(包括 plugin、scripts、config 等)也进入校验,并继续要求其匹配
allowed_dependency_prefixes;同时将 transcript_registry_dir 缺失路径产生的 ASR_LAYERING
违规分类改为 VOICE_INPUT_LAYERING,保持合同文本与实际诊断一致。
In `@static/app/app-audio-capture.js`:
- Around line 3336-3347: 在 createVoiceSettingToggle 中为隐藏的 input 补充键盘可见的焦点样式,使用
:focus-visible 将焦点环呈现在对应的可见切换控件或容器上,而不是 opacity 为 0 的 checkbox 本身。确保
togglePinnedVoicePanel 的焦点转移行为保持不变,并让降噪和资源优化等选项在 Enter/Space 打开面板后能明确显示当前焦点。
In `@static/locales/en.json`:
- Around line 936-939: 统一 asyncStatusToast 相关的 ASR 本地化文案插值变量:将 8 个 locale 中这些消息的
{{provider}} 改为 {{providerKey}},以匹配调用方传入的 providerKey;保持文案内容及其他插值变量不变。
---
Nitpick comments:
In `@main_logic/core/asr_runtime.py`:
- Around line 2136-2140: Update _dispatch_voice_input_final to capture the
result from _voice_input_registry.dispatch_final(event), import
VoiceInputDispatchResult from main_logic.voice_input, and emit a debug log when
the result is REJECTED so discarded final transcripts are observable.
In `@scripts/check_core_contracts.py`:
- Around line 512-529: Update the _dynamic_import_violations docstring to refer
to the plural forbidden_prefixes parameter concept consistently, reflecting that
the function accepts multiple forbidden prefixes while leaving the
implementation unchanged.
In `@static/app/app-settings.js`:
- Around line 1290-1312: Update optimizationWriteIsNewer for
voiceInputResourceOptimizationEnabled to use the same writerId tie-breaker as
the independentAsrEnabled arbitration logic when writeId values are equal.
Preserve the explicit-change requirement while selecting a deterministic winner
for concurrent same-id writes, so the current window’s own writer is not
overwritten by another writer.
In `@tests/unit/test_check_core_contracts.py`:
- Around line 337-368: 补充 test_run_flags_forbidden_voice_input_dependencies
的参数化输入,覆盖非白名单顶层包 plugin 的导入场景,并确认仍产生 VOICE_INPUT_LAYERING 违规。同步扩大
scripts/check_core_contracts.py 中 guarded_roots 的检查范围,使该反向依赖能够被
contract_checker.run 检测到;保留现有 main_logic、main_routers 和 utils 用例行为不变。
🪄 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: 6dd06a82-c982-487f-9355-732f01193a39
📒 Files selected for processing (36)
docs/design/smart-turn-v3-provider-neutral.mdmain_logic/asr_client/detector_runtime.pymain_logic/asr_client/runtime.pymain_logic/core/asr_runtime.pymain_logic/core/notify.pymain_logic/voice_input/__init__.pymain_logic/voice_input/consumers/__init__.pymain_logic/voice_input/consumers/core_chat.pymain_logic/voice_input/consumers/game.pymain_logic/voice_input/contracts.pymain_logic/voice_input/plugin_api.pymain_logic/voice_input/registrar.pymain_logic/voice_input/registry.pymain_logic/voice_turn/contracts.pyscripts/check_core_contracts.pystatic/app/app-audio-capture.jsstatic/app/app-settings.jsstatic/app/app-state.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/frontend/test_voice_recognition_popover.pytests/unit/test_app_websocket_static.pytests/unit/test_asr_detector_runtime.pytests/unit/test_audio_stream_queue.pytests/unit/test_check_core_contracts.pytests/unit/test_core_independent_asr.pytests/unit/test_voice_input_consumers.pytests/unit/test_voice_input_registry.pytests/unit/test_voice_recognition_settings_static.pytests/unit/voice_turn/test_contracts.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 (11)
- main_logic/voice_input/consumers/init.py
- static/locales/pt.json
- static/locales/ko.json
- static/locales/zh-CN.json
- main_logic/core/notify.py
- static/locales/es.json
- tests/unit/test_voice_input_consumers.py
- static/locales/ru.json
- static/locales/ja.json
- docs/design/smart-turn-v3-provider-neutral.md
- static/locales/zh-TW.json
Review 修复回执(最新 head
|
4c6d71d to
ac4b1ca
Compare
本轮 review 修复回执(head
|
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 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/voice_input/registry.py`:
- Around line 113-119: Ensure route removal and asynchronous cancellation are
serialized with in-progress prepare_turn callbacks: update VoiceInputRegistry
around activate and its related lifecycle methods to track or lock each route’s
pending callback, and defer terminal cancellation until that callback completes
so cancellation cannot be followed by prepare_turn restoring state. In
tests/unit/test_voice_input_registry.py lines 308-338, add assertions covering
the cancellation-before-prepare-resumption interleaving and verify both
operation order and final state.
In `@scripts/check_core_contracts.py`:
- Around line 547-560: Update _dynamic_import_target() to resolve relative
module names using the supplied package argument and to expand __import__
targets with non-empty statically known fromlist entries, so checks evaluate the
actual absolute module path. When either form cannot be inferred statically,
fail closed instead of skipping validation, and add regression tests covering
relative import_module calls and __import__ fromlist imports.
In `@static/app/app-audio-capture.js`:
- Around line 2534-2537: 修正弹窗渲染的三个边界问题:在异步失败并进入 catch 的清理路径中复用 renderGeneration
与 voiceRecognitionPopoverRenderGeneration 的代际校验,避免旧渲染清空新
DOM;将宿主销毁检测从仅观察自身属性改为观察稳定父节点的 childList,或接入宿主 dispose 流程以清理 portal 和全局监听器;为每个
toggle.input 关联对应可见标签和提示文本,补充无障碍名称与描述关联。
In `@static/app/app-settings.js`:
- Around line 60-130: 修正 _noteSettingDecision 及其调用方
_noteAsrDecision、_noteOptimizationDecision:不要仅因值相同就复用旧决议;结合当前运行时设置值区分 dirty
导致的重复保存与用户真实切换,用户从启动恢复值切换后必须生成新的 writeId/writerId
元组。同步检查相关保存/合并路径,确保新决议可被其他窗口正确判定。补充覆盖“启动恢复 false、服务器合并为 true、用户再切回 false”的回归测试。
In `@tests/frontend/test_voice_recognition_popover.py`:
- Around line 389-444: 为 root_page 增加针对 /static/app/app-audio-capture.js 的
console error 监听和 pageerror 收集,并在根路由加载断言脚本响应成功后补充空错误断言;保持现有 root_page
脚本加载与关闭流程不变。
In `@tests/unit/test_voice_input_registry.py`:
- Around line 308-338: 增强 test_switch_during_prepare_rejects_old_route_once,记录
slow_prepare 完成与 game.on_cancelled 回调的执行顺序,断言 prepare 完成后才发生取消。模拟 prepare
完成后的状态写入,并在等待空闲后验证旧路由状态已清理、最终无残留,同时保留现有返回值和仅取消一次的断言。
🪄 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: 03705a73-cded-4d01-acbe-3fa63bfc07e1
📒 Files selected for processing (33)
docs/design/smart-turn-v3-provider-neutral.mdmain_logic/asr_client/runtime.pymain_logic/core/asr_runtime.pymain_logic/core/notify.pymain_logic/voice_input/__init__.pymain_logic/voice_input/consumers/__init__.pymain_logic/voice_input/consumers/core_chat.pymain_logic/voice_input/consumers/game.pymain_logic/voice_input/contracts.pymain_logic/voice_input/plugin_api.pymain_logic/voice_input/registrar.pymain_logic/voice_input/registry.pymain_logic/voice_turn/contracts.pyscripts/check_core_contracts.pystatic/app/app-audio-capture.jsstatic/app/app-settings.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/frontend/test_voice_recognition_popover.pytests/unit/test_app_websocket_static.pytests/unit/test_audio_stream_queue.pytests/unit/test_check_core_contracts.pytests/unit/test_core_independent_asr.pytests/unit/test_voice_input_consumers.pytests/unit/test_voice_input_registry.pytests/unit/test_voice_recognition_settings_static.pytests/unit/voice_turn/test_contracts.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 (9)
- main_logic/core/notify.py
- static/locales/en.json
- static/locales/ru.json
- static/locales/ko.json
- docs/design/smart-turn-v3-provider-neutral.md
- tests/unit/test_check_core_contracts.py
- tests/unit/test_voice_input_consumers.py
- static/locales/zh-TW.json
- static/locales/ja.json
👮 Files not reviewed due to content moderation or server errors (8)
- main_logic/voice_turn/contracts.py
- main_logic/voice_input/consumers/init.py
- main_logic/voice_input/consumers/core_chat.py
- main_logic/voice_input/consumers/game.py
- main_logic/core/asr_runtime.py
- main_logic/asr_client/runtime.py
- tests/unit/test_audio_stream_queue.py
- tests/unit/test_core_independent_asr.py
ac4b1ca to
f55d67c
Compare
|
按备忘录完成本轮 review comment 分析与修复。结论:这些 comment 不是单纯“硬编码误报”,而是 5 个真实的通用语义/覆盖缺口;其中 2 条 Registry comment 属于同一个 asyncio 顺序竞态。 本轮没有加入业务特判:
所有修复已 autosquash 回原 6 个阶段提交并推送到同一 PR(HEAD 验证:完整 unit |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
tests/unit/test_voice_input_consumers.py (1)
106-125: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win给 CancelledError 路径也补一个重复取消的幂等断言喵~
test_core_consumer_rejected_prepare_retains_context_for_cancel(lines 83-104)已经验证了 prepare 返回False时重复调用on_cancelled只会消费一次(cancelled.assert_awaited_once())。但紧邻的这个test_core_consumer_cancelled_prepare_retains_context_for_cancel测试 prepare 抛出asyncio.CancelledError的路径时,只调用了一次on_cancelled,没有像姊妹测试那样再调用一次并断言仍然只消费一次,喵~两条终止路径本该有对称的覆盖,不然本喵怎么放心这条路径也是幂等的呢,哼!♻️ 建议补充的断言
with pytest.raises(asyncio.CancelledError): await consumer.prepare_turn(token) await consumer.on_cancelled(token, "prepare_cancelled") + await consumer.on_cancelled(token, "duplicate_cancel") + cancelled.assert_awaited_once() context, reason = cancelled.await_args.args assert context.token == token assert context.session_ref is session assert reason == "prepare_cancelled"🤖 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_voice_input_consumers.py` around lines 106 - 125, Update test_core_consumer_cancelled_prepare_retains_context_for_cancel to invoke consumer.on_cancelled a second time with the same token and reason, then assert cancelled.assert_awaited_once() while preserving the existing context and reason assertions.tests/frontend/test_voice_recognition_popover.py (1)
11-27: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value切片标记一旦被改动,报错会变成一句莫名其妙的 ValueError 喵。
source.index(...)依赖// 监听设备变化与/** 轻量级更新:仅更新选中状态 */这两处注释,后人重构注释时只会看到substring not found,根本猜不到要去改 harness 喵~加个显式断言把契约说清楚更友好。♻️ 建议改动
def _voice_popover_sources() -> tuple[str, str]: source = APP_AUDIO_CAPTURE.read_text(encoding="utf-8") + for marker in ( + "async function ensureMicrophonePermission()", + "// 监听设备变化", + "window.renderFloatingMicList = async function", + "/** 轻量级更新:仅更新选中状态 */", + ): + assert marker in source, ( + f"harness slice marker missing in app-audio-capture.js: {marker!r}" + ) + permission_start = source.index("async function ensureMicrophonePermission()")🤖 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/frontend/test_voice_recognition_popover.py` around lines 11 - 27, Update _voice_popover_sources to explicitly validate that the expected slicing markers—“// 监听设备变化” and “/** 轻量级更新:仅更新选中状态 */”—remain present before calling source.index, raising a clear assertion that identifies the missing contract marker instead of exposing a generic ValueError.static/app/app-settings.js (1)
987-1007: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value两条 boot 决议恢复分支的判定条件不对称喵。
ASR 只在
changedKeys含键时恢复,optimization 则额外接受bootMeta.optimizationDecision存在的情况。后者更严谨(决议元组本身就是显式证据),但两边不一致会让人以后读代码时犯迷糊喵~建议把 ASR 分支也补上bootMeta.asrDecision ||,语义统一。♻️ 建议改动
- if (bootMeta && bootMeta.changedKeys.indexOf('independentAsrEnabled') !== -1) { + if ( + bootMeta + && ( + bootMeta.asrDecision + || bootMeta.changedKeys.indexOf('independentAsrEnabled') !== -1 + ) + ) { const bootDecision = bootMeta.asrDecision || bootMeta; _noteAsrDecision(bootDecision.writeId, bootDecision.writerId, settings.independentAsrEnabled); }🤖 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-settings.js` around lines 987 - 1007, 统一两条 boot 决议恢复分支的判定条件:更新 ASR 分支,使其在 bootMeta.asrDecision 存在时也执行恢复,同时保留 changedKeys 包含 independentAsrEnabled 的现有条件;继续使用 _noteAsrDecision 的现有参数传递逻辑。main_logic/core/asr_runtime.py (1)
318-321: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
_core_asr_preview_turn_token没有在这里补兜底喵。同一族的
_core_asr_preview_turn_id/_core_asr_preview_text都在_ensure_asr_runtime_state里有hasattr兜底,唯独新加的 token 字段没有,所以_restore_core_asr_preview_after_final只能靠getattr(..., None)兜着。现在不会炸,但这种「一半靠初始化、一半靠 getattr」的写法以后很容易踩坑喵~♻️ 建议对齐现有兜底模式
if not hasattr(self, "_core_asr_preview_turn_id"): self._core_asr_preview_turn_id = "" + if not hasattr(self, "_core_asr_preview_turn_token"): + self._core_asr_preview_turn_token = None if not hasattr(self, "_core_asr_preview_text"): self._core_asr_preview_text = ""🤖 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/core/asr_runtime.py` around lines 318 - 321, 在 _ensure_asr_runtime_state 中补充对 _core_asr_preview_turn_token 的 hasattr 兜底初始化,使其与 _core_asr_preview_turn_id 和 _core_asr_preview_text 使用一致的运行时状态初始化模式,并保留现有 _init_voice_input_registry 调用不变。tests/unit/test_core_independent_asr.py (1)
1559-1564: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value断言
registry._records私有字典有点越界喵。前四条断言(对象同一性 +
closed is False)已经足够证明 builtin 注册没被 close 重建了;最后一条却把测试钉死在 registry 的内部存储结构上,将来换成别的容器就会无谓地红一片喵~ 如果确实想锁「只有两个消费者」,更建议 registry 暴露一个只读的计数/标识视图再断言喵。🤖 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_core_independent_asr.py` around lines 1559 - 1564, 移除对 registry._records 私有存储结构的断言,保留 runtime 注册表对象同一性及两个 registration 的 closed 状态断言;不要让测试依赖 registry 的具体容器实现。若必须验证消费者数量,应改用现有的公开只读计数或标识接口,勿新增私有字段依赖。
🤖 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/voice_input/registry.py`:
- Around line 185-220: Wrap the consumer callbacks in dispatch_partial and
dispatch_final with exception isolation so on_partial or on_final failures do
not propagate to the ASR dispatch task. Preserve the existing token-bound
routing and fail-closed state transitions, including consuming the final route
before invoking on_final, and return the appropriate non-delivery result after a
callback failure using the file’s established error-handling convention.
---
Nitpick comments:
In `@main_logic/core/asr_runtime.py`:
- Around line 318-321: 在 _ensure_asr_runtime_state 中补充对
_core_asr_preview_turn_token 的 hasattr 兜底初始化,使其与 _core_asr_preview_turn_id 和
_core_asr_preview_text 使用一致的运行时状态初始化模式,并保留现有 _init_voice_input_registry 调用不变。
In `@static/app/app-settings.js`:
- Around line 987-1007: 统一两条 boot 决议恢复分支的判定条件:更新 ASR 分支,使其在 bootMeta.asrDecision
存在时也执行恢复,同时保留 changedKeys 包含 independentAsrEnabled 的现有条件;继续使用 _noteAsrDecision
的现有参数传递逻辑。
In `@tests/frontend/test_voice_recognition_popover.py`:
- Around line 11-27: Update _voice_popover_sources to explicitly validate that
the expected slicing markers—“// 监听设备变化” and “/** 轻量级更新:仅更新选中状态 */”—remain
present before calling source.index, raising a clear assertion that identifies
the missing contract marker instead of exposing a generic ValueError.
In `@tests/unit/test_core_independent_asr.py`:
- Around line 1559-1564: 移除对 registry._records 私有存储结构的断言,保留 runtime 注册表对象同一性及两个
registration 的 closed 状态断言;不要让测试依赖 registry
的具体容器实现。若必须验证消费者数量,应改用现有的公开只读计数或标识接口,勿新增私有字段依赖。
In `@tests/unit/test_voice_input_consumers.py`:
- Around line 106-125: Update
test_core_consumer_cancelled_prepare_retains_context_for_cancel to invoke
consumer.on_cancelled a second time with the same token and reason, then assert
cancelled.assert_awaited_once() while preserving the existing context and reason
assertions.
🪄 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: 72cd066a-02d4-4156-a170-06e2fc3fa907
📒 Files selected for processing (33)
docs/design/smart-turn-v3-provider-neutral.mdmain_logic/asr_client/runtime.pymain_logic/core/asr_runtime.pymain_logic/core/notify.pymain_logic/voice_input/__init__.pymain_logic/voice_input/consumers/__init__.pymain_logic/voice_input/consumers/core_chat.pymain_logic/voice_input/consumers/game.pymain_logic/voice_input/contracts.pymain_logic/voice_input/plugin_api.pymain_logic/voice_input/registrar.pymain_logic/voice_input/registry.pymain_logic/voice_turn/contracts.pyscripts/check_core_contracts.pystatic/app/app-audio-capture.jsstatic/app/app-settings.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/frontend/test_voice_recognition_popover.pytests/unit/test_app_websocket_static.pytests/unit/test_audio_stream_queue.pytests/unit/test_check_core_contracts.pytests/unit/test_core_independent_asr.pytests/unit/test_voice_input_consumers.pytests/unit/test_voice_input_registry.pytests/unit/test_voice_recognition_settings_static.pytests/unit/voice_turn/test_contracts.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 (5)
- main_logic/voice_input/init.py
- main_logic/core/notify.py
- docs/design/smart-turn-v3-provider-neutral.md
- static/locales/pt.json
- static/locales/ko.json
f55d67c to
66f749c
Compare
|
补充:CodeRabbit 完成后新增的 consumer callback exception 评论也已处理。该问题是实际异常隔离缺口,不是硬编码问题。修复采用通用 CALLBACK_FAILED disposition,而不是按插件/Core/game 写特判;并保留既有 Core 脱敏失败通知。最新 HEAD 66f749c,仍严格 6 个阶段提交;完整 unit 8445 passed / 46 skipped,聚焦 372 passed,最终 GitNexus LOW / 0 affected flows。 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
tests/unit/test_check_core_contracts.py (2)
140-146: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win断文档字符串可不算测行为哦喵~
这个用例只要有人改一下措辞就会红,却证明不了多前缀真的生效;而且目前没有任何测试直接给
_dynamic_import_violations传元组前缀(helper 只传单个字符串)。建议换成真正的行为断言喵。💚 建议改动(改为多前缀行为测试)
`@pytest.mark.unit` -def test_dynamic_import_docstring_describes_multiple_forbidden_prefixes( +def test_dynamic_import_violations_match_each_forbidden_prefix( contract_checker, ) -> None: - docstring = contract_checker._dynamic_import_violations.__doc__ or "" - - assert "forbidden prefixes" in docstring + source = ( + "import importlib\n" + "importlib.import_module('main_logic.voice_input.registry')\n" + ) + tree = ast.parse(source) + aliases = contract_checker.module_alias_paths(tree, "main_logic.asr_client") + + messages = [ + violation.message + for violation in contract_checker._dynamic_import_violations( + Path("loader.py"), + tree, + aliases, + ("main_logic.core", "main_logic.voice_input"), + "asr_client", + ) + ] + + assert messages == [ + "asr_client must not import main_logic.voice_input (dynamic import)" + ]🤖 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_check_core_contracts.py` around lines 140 - 146, Replace test_dynamic_import_docstring_describes_multiple_forbidden_prefixes with a behavior-focused test for contract_checker._dynamic_import_violations: pass a tuple containing multiple forbidden prefixes and assert imports matching each prefix are reported as violations, while an unrelated import is not. Remove the docstring-content assertion and preserve the helper’s expected violation result format.
405-412: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
assert messages太宽松了,闸门报错原因错了它也照样绿喵~参数化里已经明确了每个 source 的越界模块,顺手把它一起断言进去,才能防止「报了违规但指错模块」的回归喵。
♻️ 建议改动(参数化期望模块)
-@pytest.mark.parametrize( - "source", - [ - "import main_logic.core\n", +@pytest.mark.parametrize( + ("source", "expected_module"), + [ + ("import main_logic.core\n", "main_logic.core"), ... ], )并把断言收紧为
assert any(expected_module in message for message in messages)。🤖 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_check_core_contracts.py` around lines 405 - 412, 收紧 `VOICE_INPUT_LAYERING` 测试中的断言:使用参数化用例已定义的 `expected_module`,改为断言 `messages` 中至少有一条消息包含该期望模块。保留现有违规筛选逻辑,确保不仅检测到违规,还验证报告指向正确模块。scripts/check_core_contracts.py (1)
584-597: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value这个 helper 已经通用化了,
code却还钉死在ASR_LAYERING上喵~因为 code 不可配置,1217-1234 的 voice_input 动态导入检查只能把
_dynamic_import_target的 target/detail 逻辑再抄一遍。把 code 提成参数就能让两边共用同一条闸门,以后加新分层也不用再复制粘贴喵。♻️ 建议改动(参数化 violation code)
def _dynamic_import_violations( path: Path, tree: ast.Module, alias_paths: dict[str, str], forbidden_prefix: str | tuple[str, ...], where: str, + code: str = "ASR_LAYERING", ) -> list["Violation"]:随后把两处
Violation(..., "ASR_LAYERING", ...)改为使用code,voice_input 侧即可直接调用并传入"VOICE_INPUT_LAYERING"。🤖 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 `@scripts/check_core_contracts.py` around lines 584 - 597, 将 _dynamic_import_violations 增加可配置的 violation code 参数,并在该 helper 内两处构造 Violation 的位置改用 code,而不是写死 "ASR_LAYERING"。同步更新现有调用方传入 "ASR_LAYERING",让 voice_input 动态导入检查复用此 helper 并传入 "VOICE_INPUT_LAYERING",保持现有 target/detail 校验逻辑不变。main_logic/core/asr_runtime.py (2)
2508-2510: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win在
_asr_notification_lock里await wait_idle(),取消回调将来一旦发状态就会自锁喵。
wait_idle()会把 registry 的 cancellation task 跑完,也就是在持锁期间执行_cancel_core_chat_voice_turn。现在它只做_send_core_asr_preview_clear(直写 websocket)+同步 abandon,所以不会重入这把锁、暂时安全喵。但_send_core_asr_status/_send_core_asr_lifecycle都要拿同一把锁,哪天有人往 cancel 回调里加一句状态推送,这里就直接死锁给你看喵——建议在这行加一句「cancel 回调禁止走_send_core_asr_*」的不变量注释,或者把wait_idle()挪到锁外面啦喵~🤖 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/core/asr_runtime.py` around lines 2508 - 2510, 在持有 _asr_notification_lock 的流程中移除 await self._voice_input_registry.wait_idle(),并将其移动到释放该锁之后执行;确保取消任务仍会等待完成,同时避免取消回调重入同一把锁。若现有控制流无法安全移动,则在该调用处补充明确不变量注释,说明取消回调禁止调用 _send_core_asr_*。
259-261: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuepreview 三件套只初始化了两个,
_core_asr_preview_turn_token被落下了喵。
_core_asr_preview_turn_id/_core_asr_preview_text都在这里(以及_ensure_asr_runtime_state的 Line 336-339)建好了,但同属一组的_core_asr_preview_turn_token只在_prepare_core_voice_turn(Line 2192)第一次被赋值;Line 2435 因此只能用getattr(..., None)兜着喵。现在不会炸,可这个不对称状态迟早会有人直接self._core_asr_preview_turn_token一读就AttributeError啦,顺手补齐比留个 getattr 拐杖干净多啦喵~♻️ 建议补齐初始化喵
self._core_asr_preview_turn_id = "" + self._core_asr_preview_turn_token: VoiceTurnToken | None = None self._core_asr_preview_text = ""另外在
_ensure_asr_runtime_state里同步补一条(Line 336-339 附近):if not hasattr(self, "_core_asr_preview_turn_token"): self._core_asr_preview_turn_token = None之后 Line 2435 的
getattr就可以直接换成属性读取啦喵。🤖 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/core/asr_runtime.py` around lines 259 - 261, Initialize _core_asr_preview_turn_token alongside _core_asr_preview_turn_id and _core_asr_preview_text in the runtime setup, and add the same missing-attribute guard in _ensure_asr_runtime_state using None. Then replace the defensive getattr access near the preview handling path with direct self._core_asr_preview_turn_token access.
🤖 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 `@scripts/check_core_contracts.py`:
- Around line 1081-1090: Update the violation message in the
VOICE_INPUT_LAYERING path-validation branch to say “voice input layering”
instead of “ASR layering,” while preserving the existing violation code and
missing-path behavior.
---
Nitpick comments:
In `@main_logic/core/asr_runtime.py`:
- Around line 2508-2510: 在持有 _asr_notification_lock 的流程中移除 await
self._voice_input_registry.wait_idle(),并将其移动到释放该锁之后执行;确保取消任务仍会等待完成,同时避免取消回调重入同一把锁。若现有控制流无法安全移动,则在该调用处补充明确不变量注释,说明取消回调禁止调用
_send_core_asr_*。
- Around line 259-261: Initialize _core_asr_preview_turn_token alongside
_core_asr_preview_turn_id and _core_asr_preview_text in the runtime setup, and
add the same missing-attribute guard in _ensure_asr_runtime_state using None.
Then replace the defensive getattr access near the preview handling path with
direct self._core_asr_preview_turn_token access.
In `@scripts/check_core_contracts.py`:
- Around line 584-597: 将 _dynamic_import_violations 增加可配置的 violation code 参数,并在该
helper 内两处构造 Violation 的位置改用 code,而不是写死 "ASR_LAYERING"。同步更新现有调用方传入
"ASR_LAYERING",让 voice_input 动态导入检查复用此 helper 并传入 "VOICE_INPUT_LAYERING",保持现有
target/detail 校验逻辑不变。
In `@tests/unit/test_check_core_contracts.py`:
- Around line 140-146: Replace
test_dynamic_import_docstring_describes_multiple_forbidden_prefixes with a
behavior-focused test for contract_checker._dynamic_import_violations: pass a
tuple containing multiple forbidden prefixes and assert imports matching each
prefix are reported as violations, while an unrelated import is not. Remove the
docstring-content assertion and preserve the helper’s expected violation result
format.
- Around line 405-412: 收紧 `VOICE_INPUT_LAYERING` 测试中的断言:使用参数化用例已定义的
`expected_module`,改为断言 `messages`
中至少有一条消息包含该期望模块。保留现有违规筛选逻辑,确保不仅检测到违规,还验证报告指向正确模块。
🪄 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: 4add1a39-4983-4cfc-8df3-e6618ec33b0e
📒 Files selected for processing (17)
docs/design/smart-turn-v3-provider-neutral.mdmain_logic/core/asr_runtime.pymain_logic/core/notify.pymain_logic/voice_input/__init__.pymain_logic/voice_input/consumers/__init__.pymain_logic/voice_input/consumers/core_chat.pymain_logic/voice_input/consumers/game.pymain_logic/voice_input/contracts.pymain_logic/voice_input/plugin_api.pymain_logic/voice_input/registrar.pymain_logic/voice_input/registry.pyscripts/check_core_contracts.pytests/unit/test_audio_stream_queue.pytests/unit/test_check_core_contracts.pytests/unit/test_core_independent_asr.pytests/unit/test_voice_input_consumers.pytests/unit/test_voice_input_registry.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 (3)
- main_logic/voice_input/consumers/init.py
- main_logic/core/notify.py
- docs/design/smart-turn-v3-provider-neutral.md
最新 review 修复说明(head
|
|
@codex review |
|
@greptil review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cd4c0ce698
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Review comments 修复回归说明(bfbc7eda9)本次集中处理 3 个 review thread:
回归范围:
兼容边界:旧前端或缺失/畸形握手字段仍回退到已持久化的后端设置;本次未改变设置 API schema、语音协议 action 或非语音会话行为。 |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
static/app/app-audio-capture.js (1)
3452-3453: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
disposeVoiceRecognitionPopover赋值晚了一步,createVoicePanel()中途炸了监听器就收不回来了喵!
createVoicePanel()内部会注册pointerdown/keydown/resize/scroll/voice-input-lifecycle-changed/neko:voice-session-started/neko:voice-settings-pending-changed一串 document/window 监听器(Line 3132-3151),直到函数末尾updateVoiceRecognitionUi()(Line 3166)才 return。要是这中间随便哪一步抛了异常,外层try/catch(Line 3979-3986)这时候disposeVoiceRecognitionPopover还是null,压根不会去调用destroyVoicePanel(),那些已经注册好的监听器就永远挂在 document/window 上收不回来了喵!
destroyVoicePanel()对没注册过的监听器调用removeEventListener本来就是安全的空操作,voicePanel/voiceBridge判空也稳,所以只要把赋值提前到createVoicePanel()调用之前就行,一点风险都不用担心喵~🐛 建议改动
- createVoicePanel(); - disposeVoiceRecognitionPopover = destroyVoicePanel; + disposeVoiceRecognitionPopover = destroyVoicePanel; + createVoicePanel();🤖 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-audio-capture.js` around lines 3452 - 3453, Move the assignment of disposeVoiceRecognitionPopover to destroyVoicePanel before calling createVoicePanel(). Keep the surrounding initialization and cleanup flow unchanged so the existing error handler can invoke destroyVoicePanel if createVoicePanel fails during listener registration or UI setup.main_logic/core/asr_runtime.py (1)
1793-1844: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win_apply_voice_lease_state() 需要先推进 ASR generation barrier,再 wait_idle() 哦喵
abort/suspend()会先执行生命周期和 transport 层面的无效化/断连,但不会像_abort_independent_asr()那样预先推进 generation barrier;现在game_release、owner==game和should_abort这几处都是先wait_idle(),挂起期间可能还有当前入口的旧 token/旧 generation PCM 继续送出去喵。改成先执行asr_runtime.abort()/suspend()+ PCM sync invalidation,再等待语音输入空闲喵。🤖 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/core/asr_runtime.py` around lines 1793 - 1844, 调整_apply_voice_lease_state()中的生命周期顺序:在owner为game、game_release以及通用should_abort分支中,先调用_asr_runtime的suspend()/abort()推进ASR generation barrier并完成PCM同步失效,再调用_voice_input_registry.wait_idle()。保留现有game_release的generation/owner校验及后续resume逻辑,确保旧generation的PCM不会在等待输入空闲期间继续发送。
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/frontend/test_voice_recognition_popover.py`:
- Around line 269-301: Extend the listener-balance key list in
test_current_voice_popover_failure_disposes_owned_portal to include
window:neko:voice-settings-pending-changed, ensuring the failure-disposal test
also asserts this popover listener is removed.
In `@tests/unit/test_websocket_binary_audio.py`:
- Around line 369-379: 更新该测试中围绕 manager.calls 的握手顺序断言:不要使用只返回首次匹配位置的
call_names.index;分别收集两次 start_session 及对应握手调用的索引,并逐项断言每次启动的 asr_handshake 和
resource_optimization_handshake 都发生在对应 start_session 之前。
---
Outside diff comments:
In `@main_logic/core/asr_runtime.py`:
- Around line 1793-1844:
调整_apply_voice_lease_state()中的生命周期顺序:在owner为game、game_release以及通用should_abort分支中,先调用_asr_runtime的suspend()/abort()推进ASR
generation
barrier并完成PCM同步失效,再调用_voice_input_registry.wait_idle()。保留现有game_release的generation/owner校验及后续resume逻辑,确保旧generation的PCM不会在等待输入空闲期间继续发送。
In `@static/app/app-audio-capture.js`:
- Around line 3452-3453: Move the assignment of disposeVoiceRecognitionPopover
to destroyVoicePanel before calling createVoicePanel(). Keep the surrounding
initialization and cleanup flow unchanged so the existing error handler can
invoke destroyVoicePanel if createVoicePanel fails during listener registration
or UI setup.
🪄 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: 815cb042-d098-4705-921c-89f25a73ff35
📒 Files selected for processing (13)
main_logic/core/asr_runtime.pymain_logic/core/lifecycle.pymain_routers/websocket_router.pystatic/app/app-audio-capture.jsstatic/app/app-settings.jsstatic/app/app-state.jsstatic/app/app-websocket.jstests/frontend/test_voice_recognition_popover.pytests/unit/test_app_websocket_static.pytests/unit/test_audio_stream_queue.pytests/unit/test_core_independent_asr.pytests/unit/test_voice_recognition_settings_static.pytests/unit/test_websocket_binary_audio.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)
| @pytest.mark.frontend | ||
| def test_current_voice_popover_failure_disposes_owned_portal(page: Page) -> None: | ||
| _install_voice_popover_harness(page, deferred_permission=False) | ||
|
|
||
| result = page.evaluate( | ||
| """async () => { | ||
| const popup = window.__voicePopoverTest.popup(); | ||
| window.__voicePopoverTest.failMicVolumeVisualization(); | ||
| const rendered = await window.renderFloatingMicList(popup); | ||
| return { | ||
| rendered, | ||
| panels: window.__voicePopoverTest.panels(), | ||
| errorText: popup.textContent, | ||
| listenerBalance: { | ||
| ...window.__voicePopoverTest.listenerBalance, | ||
| }, | ||
| }; | ||
| }""" | ||
| ) | ||
|
|
||
| assert result["rendered"] is True | ||
| assert result["panels"] == 0 | ||
| assert result["errorText"] == "microphone.loadFailed" | ||
| for key in ( | ||
| "document:pointerdown", | ||
| "document:keydown", | ||
| "window:resize", | ||
| "window:scroll", | ||
| "window:voice-input-lifecycle-changed", | ||
| "window:neko:voice-session-started", | ||
| ): | ||
| assert result["listenerBalance"].get(key) == 0 | ||
|
|
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
监听器归零清单里漏了新增的 neko:voice-settings-pending-changed 喵,补上才算真的锁死喵!
这条测试就是专门用来锁定"渲染失败后所有 popover 监听器都要归零"这个不变量的,可是循环里的 key 列表还是老六件套,本 PR 新加的 window:neko:voice-settings-pending-changed(createVoicePanel/destroyVoicePanel 对称注册/移除的那个)没被列进去喵。代码大概率是对的,但测试没盯住它,以后这个监听器的清理万一被改坏了也不会被这条测试抓出来喵~
✅ 建议补充
for key in (
"document:pointerdown",
"document:keydown",
"window:resize",
"window:scroll",
"window:voice-input-lifecycle-changed",
"window:neko:voice-session-started",
+ "window:neko:voice-settings-pending-changed",
):
assert result["listenerBalance"].get(key) == 0📝 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.
| @pytest.mark.frontend | |
| def test_current_voice_popover_failure_disposes_owned_portal(page: Page) -> None: | |
| _install_voice_popover_harness(page, deferred_permission=False) | |
| result = page.evaluate( | |
| """async () => { | |
| const popup = window.__voicePopoverTest.popup(); | |
| window.__voicePopoverTest.failMicVolumeVisualization(); | |
| const rendered = await window.renderFloatingMicList(popup); | |
| return { | |
| rendered, | |
| panels: window.__voicePopoverTest.panels(), | |
| errorText: popup.textContent, | |
| listenerBalance: { | |
| ...window.__voicePopoverTest.listenerBalance, | |
| }, | |
| }; | |
| }""" | |
| ) | |
| assert result["rendered"] is True | |
| assert result["panels"] == 0 | |
| assert result["errorText"] == "microphone.loadFailed" | |
| for key in ( | |
| "document:pointerdown", | |
| "document:keydown", | |
| "window:resize", | |
| "window:scroll", | |
| "window:voice-input-lifecycle-changed", | |
| "window:neko:voice-session-started", | |
| ): | |
| assert result["listenerBalance"].get(key) == 0 | |
| `@pytest.mark.frontend` | |
| def test_current_voice_popover_failure_disposes_owned_portal(page: Page) -> None: | |
| _install_voice_popover_harness(page, deferred_permission=False) | |
| result = page.evaluate( | |
| """async () => { | |
| const popup = window.__voicePopoverTest.popup(); | |
| window.__voicePopoverTest.failMicVolumeVisualization(); | |
| const rendered = await window.renderFloatingMicList(popup); | |
| return { | |
| rendered, | |
| panels: window.__voicePopoverTest.panels(), | |
| errorText: popup.textContent, | |
| listenerBalance: { | |
| ...window.__voicePopoverTest.listenerBalance, | |
| }, | |
| }; | |
| }""" | |
| ) | |
| assert result["rendered"] is True | |
| assert result["panels"] == 0 | |
| assert result["errorText"] == "microphone.loadFailed" | |
| for key in ( | |
| "document:pointerdown", | |
| "document:keydown", | |
| "window:resize", | |
| "window:scroll", | |
| "window:voice-input-lifecycle-changed", | |
| "window:neko:voice-session-started", | |
| "window:neko:voice-settings-pending-changed", | |
| ): | |
| assert result["listenerBalance"].get(key) == 0 |
🤖 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/frontend/test_voice_recognition_popover.py` around lines 269 - 301,
Extend the listener-balance key list in
test_current_voice_popover_failure_disposes_owned_portal to include
window:neko:voice-settings-pending-changed, ensuring the failure-disposal test
also asserts this popover listener is removed.
| assert [ | ||
| payload | ||
| for name, payload in manager.calls | ||
| if name == "resource_optimization_handshake" | ||
| ] == [False, None] | ||
| call_names = [name for name, _payload in manager.calls] | ||
| assert call_names.index("asr_handshake") < call_names.index("start_session") | ||
| assert ( | ||
| call_names.index("resource_optimization_handshake") | ||
| < call_names.index("start_session") | ||
| ) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
请按每次启动逐项校验握手顺序喵。
当前 .index() 只检查第一次出现;即使第二次握手错误地发生在第二个 start_session 之后,测试仍会通过。请收集两次调用的索引并逐项比较,锁住“每个启动请求都先写入对应握手”的契约喵。
建议修复喵
- assert call_names.index("resource_optimization_handshake") < call_names.index("start_session")
+ start_indices = [i for i, name in enumerate(call_names) if name == "start_session"]
+ resource_indices = [
+ i for i, name in enumerate(call_names)
+ if name == "resource_optimization_handshake"
+ ]
+ assert all(handshake < start for handshake, start in zip(resource_indices, start_indices))📝 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.
| assert [ | |
| payload | |
| for name, payload in manager.calls | |
| if name == "resource_optimization_handshake" | |
| ] == [False, None] | |
| call_names = [name for name, _payload in manager.calls] | |
| assert call_names.index("asr_handshake") < call_names.index("start_session") | |
| assert ( | |
| call_names.index("resource_optimization_handshake") | |
| < call_names.index("start_session") | |
| ) | |
| assert [ | |
| payload | |
| for name, payload in manager.calls | |
| if name == "resource_optimization_handshake" | |
| ] == [False, None] | |
| call_names = [name for name, _payload in manager.calls] | |
| assert call_names.index("asr_handshake") < call_names.index("start_session") | |
| start_indices = [i for i, name in enumerate(call_names) if name == "start_session"] | |
| resource_indices = [ | |
| i for i, name in enumerate(call_names) | |
| if name == "resource_optimization_handshake" | |
| ] | |
| assert all(handshake < start for handshake, start in zip(resource_indices, start_indices)) |
🤖 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_websocket_binary_audio.py` around lines 369 - 379, 更新该测试中围绕
manager.calls 的握手顺序断言:不要使用只返回首次匹配位置的 call_names.index;分别收集两次 start_session
及对应握手调用的索引,并逐项断言每次启动的 asr_handshake 和 resource_optimization_handshake 都发生在对应
start_session 之前。
|
@codex review |
改动概述 / Summary
Phase 4A 已按最新
main重新实现,不复用旧 PR 的 Core/runtime 历史。该 PR 只收敛语音识别设置、turn identity、受控 transcript consumer registry,以及 Core/game 内建接线。共 6 个阶段提交、36 个文件:
fix(voice): align ASR defaults and optimization semanticsfeat(voice): add shared recognition settings popoverrefactor(voice): attach turn identity to partial transcriptsfeat(voice): add controlled transcript consumer registryfeat(asr): route Core and game transcripts through registrytest(voice): enforce cancellation and empty-final barriers冻结语义
false保持不变。VoicePartialEvent和 final 使用完整VoiceTurnToken;任一 lease、route 或 audio generation 不匹配时都 fail-closed。VoiceInputRegistry与LLMSessionManager同生命周期,内建core_chat与gameconsumer 只注册一次。abandon(None)。依赖与硬边界
voice_input依赖方向合同:仅允许依赖自身包、voice_turn.contracts与utils.game_route_state;同时禁止asr_client反向导入voice_input。静态和动态 import 都覆盖。utils.game_route_state边界完成。回归报告 / Regression Report
VoiceInputRegistry,完成 Core/game consumer 与 MicLease owner 接线,并补齐默认设置、共享 popover 和依赖方向合同。VoiceTurnToken与 consumer;所有事件按同一 identity barrier 校验,final/empty-final 只终止一次,切换或不可用路径直接丢弃且不回退;Core preview/turn 按原 token 精确清理。不拆分理由 / Why Not Split
该改动按 6 个独立提交分阶段审阅,但必须作为一个原子 PR 合入:设置默认值、前端入口、turn token 合同、registry、Core/game 接线和竞态/结构合同共同定义同一 fail-closed 语义。拆成可独立合入的 PR 会在中间状态留下 partial 无身份、registry 未接线或取消屏障不完整的组合。36 个文件中还包含 8 个同步 locale 文件、新增模块和集中测试,实际生产改动仍保持在 Phase 4A 冻结边界内。
测试 / Testing
8445 passed, 46 skipped;rebase 后 Phase 4A 聚焦回归:494 passed。8 passed;i18n sync gate 通过。215 passed、audio stream queue119 passed、Registry24 passed、preview 专项14 passed。node --check、git diff --check均通过。detect_changes(upstream/main...HEAD):风险LOW,545 个映射符号、0 条受影响执行流;禁止路径命中 0。Summary by CodeRabbit
新功能
改进
测试