feat: add study companion notebook and voice interaction [Part 2/2]#1613
feat: add study companion notebook and voice interaction [Part 2/2]#1613MomiJiSan wants to merge 15 commits into
Conversation
Walkthrough引入语音转写桥接与仲裁;新增 Notebook 数据模型/FTS/NotebookStore 与插件入口;实现 Tutor LLM 的 expand/summarize note 能力;重构 Awareness/OCR 与屏幕分类;前端新增 math-parser/KaTeX 接入并增加笔记 UI 与样式;补充大量测试与 i18n 文案,喵。 Changes伴学笔记与语音桥接
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes Possibly related PRs
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f22cadf1a6
ℹ️ 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".
| @dataclass(slots=True) | ||
| class StudyConfig: |
There was a problem hiding this comment.
Keep awareness model types while callers remain
Removing the awareness dataclasses here leaves existing in-repo callers broken: plugin/plugins/study_companion/awareness_buffer.py still imports ActivitySnapshot/ActivitySummary, and the unchanged study companion tests still import AwarenessConfig and call StudyOcrPipeline.capture_lightweight. In contexts that collect those tests or import the awareness buffer module, this now fails during import instead of exercising the new notebook code, so either the remaining awareness module/tests need to be removed/updated or these compatibility types need to stay until their callers are migrated.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
plugin/plugins/study_companion/constants.py (1)
15-26:⚠️ Potential issue | 🔴 Critical | ⚡ Quick win新增的 LLM 操作常量未注册到支持集合中喵!
检测到你在 15-16 行新增了
LLM_OPERATION_EXPAND_NOTE和LLM_OPERATION_SUMMARIZE_TO_NOTE两个操作常量,但是!它们居然没有被添加到 18-26 行的SUPPORTED_LLM_OPERATIONSfrozenset 里面喵!如果代码中有地方用这个集合来验证操作是否合法(比如检查
operation in SUPPORTED_LLM_OPERATIONS),那新的笔记操作就会被拒绝,导致运行时错误喵~🐱 修复建议喵
SUPPORTED_LLM_OPERATIONS = frozenset( { LLM_OPERATION_CONCEPT_EXPLAIN, LLM_OPERATION_QUESTION_GENERATE, LLM_OPERATION_ANSWER_EVALUATE, LLM_OPERATION_KNOWLEDGE_TRACK, LLM_OPERATION_SUMMARIZE_SESSION, + LLM_OPERATION_EXPAND_NOTE, + LLM_OPERATION_SUMMARIZE_TO_NOTE, } )🤖 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 `@plugin/plugins/study_companion/constants.py` around lines 15 - 26, You added new constants LLM_OPERATION_EXPAND_NOTE and LLM_OPERATION_SUMMARIZE_TO_NOTE but didn’t include them in the SUPPORTED_LLM_OPERATIONS frozenset; update SUPPORTED_LLM_OPERATIONS to include LLM_OPERATION_EXPAND_NOTE and LLM_OPERATION_SUMMARIZE_TO_NOTE so membership checks (e.g., operation in SUPPORTED_LLM_OPERATIONS) will recognize these new operations.plugin/plugins/study_companion/tutor_llm_agent.py (1)
480-524:⚠️ Potential issue | 🟠 Major确认:
summary分组可用,但tutor分组目前会走“未知模型类型”降级喵。
utils/config_manager.py:get_model_api_config()的model_type_mapping里没有tutor(只包含conversation/summary/correction/emotion/vision/agent/realtime/tts_*),因此tutor_llm_agent_notebook.py传入model_group_override="tutor"会触发ValueError,而tutor_llm_agent.py里 518-523 的回退逻辑只覆盖“配置缺 base_url/model”,不覆盖“model_type 不存在”。喵- 结果是
expand_note()会直接进入except分支,返回_fallback_expand_note(...)(degraded=True),不会真正调用 Tutor 模型。喵- 建议:要么把
model_group_override="tutor"改成已有类型(如agent/conversation),要么在get_model_api_config()增加tutor到现有字段的映射,并补充文档说明用途喵。🤖 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 `@plugin/plugins/study_companion/tutor_llm_agent.py` around lines 480 - 524, The code assumes config_manager.get_model_api_config(model_group) will always succeed for model_group from model_group_override (e.g., "tutor"), but utils.config_manager.get_model_api_config has no "tutor" mapping and will raise ValueError causing a silent degradation; fix by catching that case where api_config is requested for requested_model_group or for model_group selection in tutor_llm_agent.py: wrap calls to config_manager.get_model_api_config(requested_group) / config_manager.get_model_api_config(model_group) in a try/except that catches the unknown-model-type ValueError and then falls back to config_manager.get_model_api_config("agent") (and set model_group="agent" and call_type_group accordingly), or alternatively add a "tutor" entry to the model_type_mapping inside utils.config_manager.get_model_api_config if the intent is to support tutor—choose the approach consistent with product intent.
🧹 Nitpick comments (6)
plugin/plugins/study_companion/entry_neko_commands.py (1)
277-277: 💤 Low value建议统一错误消息的字符串格式化方式喵~
这两行改用了字符串拼接(
+)来构造错误消息,但同一文件的第 302 行还在用 f-string(f"unclassified command: {cmd}"),而且整个文件其他地方都大量使用 f-string 喵。这种风格不一致会让代码看起来不够优雅,维护起来也麻烦喵!建议统一使用 f-string,这样代码风格更一致,可读性也更好喵~
✨ 建议的统一格式
- return Err(SdkError("unknown command: " + cmd)) + return Err(SdkError(f"unknown command: {cmd}"))- return Err(SdkError("handler not found: " + handler_name)) + return Err(SdkError(f"handler not found: {handler_name}"))Also applies to: 282-282
🤖 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 `@plugin/plugins/study_companion/entry_neko_commands.py` at line 277, The return error messages use string concatenation in the Err(SdkError("unknown command: " + cmd)) (and the similar case at line 282) which is inconsistent with the file's f-string style (e.g., f"unclassified command: {cmd}"); replace those concatenations with f-strings so the calls become Err(SdkError(f"unknown command: {cmd}")) to match the rest of the file and improve consistency for the error messages referencing cmd.plugin/plugins/study_companion/surfaces/notebook_panel.tsx (1)
115-123: ⚡ Quick win搜索没有防抖,每敲一个字都要打两次后端,太浪费了喵~
query一变useEffect就触发refresh,而refresh每次都会并行拉取study_notebook_list和study_note_list。问题有两个喵:
- 用户在搜索框里每输入一个字符就发起两次插件调用,没有防抖(debounce)。
- 笔记本列表其实跟
query无关,却在每次查询变化时被重复拉取,纯属白干喵。加个小小的 debounce 就能明显减压啦,笨蛋主人才会让后端这么累喵:
♻️ 给查询副作用加上防抖
useEffect(() => { const controller = new AbortController(); - refresh(controller.signal, selectedNotebookId, query).catch((error) => { - if (!controller.signal.aborted) { - setStatus(errorMessage(error)); - } - }); - return () => controller.abort(); + const timer = setTimeout(() => { + refresh(controller.signal, selectedNotebookId, query).catch((error) => { + if (!controller.signal.aborted) { + setStatus(errorMessage(error)); + } + }); + }, 250); + return () => { + clearTimeout(timer); + controller.abort(); + }; }, [selectedNotebookId, query]);如果想更彻底,可以把笔记本列表拆成只在挂载/创建笔记本后刷新,不要跟着
query一起拉喵。🤖 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 `@plugin/plugins/study_companion/surfaces/notebook_panel.tsx` around lines 115 - 123, The effect watching query currently calls refresh immediately so every keystroke triggers parallel fetches for study_notebook_list and study_note_list; debounce the query before invoking refresh (create a debouncedQuery via a useDebounce hook or an internal setTimeout/clearTimeout in a useEffect) and change the dependency of the refresh-calling effect to [selectedNotebookId, debouncedQuery]; also avoid refetching the notebook list on query changes by splitting refresh into two callers or adding a flag so that refresh only fetches study_note_list when invoked due to debouncedQuery, while notebook fetching is done once on mount or after notebook-create (update places referencing refresh, study_notebook_list, study_note_list, selectedNotebookId, and query accordingly).plugin/plugins/study_companion/store_schema.py (1)
191-200: FTS5可用性:目前运行时已支持 FTS5(sqlite_version=3.40.1,compile_options 包含 ENABLE_FTS5),CREATE VIRTUAL TABLE ... USING fts5在这里能成功,因此初始化阶段不应因缺少 FTS5 直接失败喵。若要覆盖可能裁剪/缺 FTS5 的极端目标平台,可把该段 CREATE 包一层 try/except,失败时再走 LIKE 回退逻辑更稳喵。🤖 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 `@plugin/plugins/study_companion/store_schema.py` around lines 191 - 200, Wrap the FTS5 CREATE statement (the conn.execute call that creates the notes_fts virtual table) in a try/except so initialization does not abort on platforms where FTS5 is unavailable; on exception, log or record the failure and fall back to the existing LIKE-based full‑text fallback path (the code path that uses plain LIKE searches against the notes table) so consumers can continue to operate without FTS5.plugin/plugins/study_companion/tutor_llm_agent_notebook.py (1)
198-203: ⚡ Quick win降级提示消息硬编码为中文喵!
发现
_fallback_expand_note的降级消息(line 202)直接硬编码了中文文案喵:"> 暂时无法连接模型扩写。建议补充定义、例子、易错点和一个自测问题。"这会导致非中文用户在模型调用失败时看到中文提示,体验不太好喵~考虑到:
expand_note函数可以访问self._config.languagesummarize_to_note已经使用了_localized_note_headings来本地化标题- 其他降级路径(如
_fallback_summary_note)虽然也有类似问题,但至少使用了本地化的 headings建议将这条消息也本地化处理,或者至少用英文作为默认值喵~
💡 本地化建议喵
可以参考现有的
study_i18n_t模式,或者简单地根据语言提供不同版本:def _fallback_expand_note(original: str, language: str = "en") -> str: messages = { "zh": "暂时无法连接模型扩写。建议补充定义、例子、易错点和一个自测问题。", "zh-cn": "暂时无法连接模型扩写。建议补充定义、例子、易错点和一个自测问题。", "zh-tw": "暫時無法連接模型擴寫。建議補充定義、例子、易錯點和一個自測問題。", "en": "Unable to connect to the model for expansion. Consider adding definitions, examples, common mistakes, and a self-test question.", "ja": "モデルへの接続ができません。定義、例、よくある間違い、自己テストの質問を追加することを検討してください。", } normalized = str(language or "").strip().lower() message = messages.get(normalized, messages["en"]) return f"{original.strip()}\n\n> [!ai]\n> {message}".strip()然后在
expand_note中传入self._config.language喵~🤖 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 `@plugin/plugins/study_companion/tutor_llm_agent_notebook.py` around lines 198 - 203, The fallback expand message in _fallback_expand_note is hardcoded to Chinese; update it to be localized (mirror how summarize_to_note uses _localized_note_headings) by accepting a language parameter or reading self._config.language from expand_note and selecting a message map (e.g., using the existing study_i18n_t pattern or a small dict keyed by normalized language codes with an English default); ensure expand_note passes the language into _fallback_expand_note (or let the method access self._config.language) so non-Chinese users see the appropriate fallback text.plugin/plugins/study_companion/surfaces/note_editor.tsx (1)
45-69: 💤 Low value预览里的列表项是“孤儿”
<li>喵。连续的
-会渲染成一串裸<li>,外面没有<ul>包着,语义不完整、列表符号也可能不显示哦。不过它能自行降级渲染,不影响功能,所以本喵就睁一只眼闭一只眼,要不要补个<ul>包裹就看你心情啦,哼喵~🤖 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 `@plugin/plugins/study_companion/surfaces/note_editor.tsx` around lines 45 - 69, The preview currently returns isolated <li> elements for lines starting with '- ' (in function markdownPreview), producing orphaned list items; update markdownPreview to group consecutive '- ' lines into a single <ul> wrapper instead of returning individual <li> nodes: iterate through lines, accumulate consecutive list-item strings into a buffer, flush that buffer as one <ul key=...>{buffer.map((txt,i)=><li key=...>{txt}</li>)}</ul> when a non-list line or end is reached, and handle other line types (headers, blockquote, blank, paragraph) as before; ensure unique keys (using index and slice) are applied to the generated <ul> and <li> elements.plugin/plugins/study_companion/surfaces/study_panel.tsx (1)
177-197: ⚡ Quick winKaTeX 直接改写
data-study-mathspan 子树:建议避免math-${index}index key 喵
plugin/plugins/study_companion/surfaces/study_panel.tsx里renderMathSpans会在useEffect([text, mathParserReady])中对所有data-study-math="true"的<span>调用katex.render(...)覆写其 DOM;同时MathReply里这些 math span 的 key 使用key={\math-${index}`},在text变化重渲染时可能导致 React 复用同一个 span 再去更新其子节点,而该子节点已被外部库改写,存在渲染不稳定/错位风险喵。 建议让 math span 的 key 与part.value` 绑定,降低复用概率喵。♻️ 建议改动
- <span - key={`math-${index}`} + <span + key={`math-${index}-${part.value}`} data-study-math="true" data-display={part.display ? 'true' : 'false'} data-math={part.value} >🤖 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 `@plugin/plugins/study_companion/surfaces/study_panel.tsx` around lines 177 - 197, renderMathSpans mutates the DOM nodes KaTeX renders, but MathReply uses index-based keys like key={`math-${index}`} causing React to reuse/mismatch spans; change the key on math spans in the MathReply component to derive from the math content (e.g., part.value or a stable hash of it) instead of the loop index so React will recreate the span when its math changes, reducing reuse of externally-mutated DOM; update references where key={`math-${index}`} is used to use key={part.value} (or a stable content-derived key) and keep renderMathSpans and its safety checks unchanged.
🤖 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 `@plugin/plugins/study_companion/entry_notebook.py`:
- Around line 361-376: The handler currently calls self._agent.expand_note even
when both note_id and content are empty; add a pre-check (before calling
self._agent.expand_note and after resolving source_content from note_id) to
return Err(SdkError("missing text", code="MISSING_TEXT")) when source_content is
empty (and no note content found); use the same error form as
study_note_ai_generate, reference SdkError, self._notebook_store.get_note, and
self._agent.expand_note so the call is short-circuited and model quota is not
consumed.
In `@plugin/plugins/study_companion/i18n/ja.json`:
- Around line 199-251: The values for many i18n keys (e.g.
entries.notebook_create, entries.note_upsert, ui.surface.note_editor,
ui.button.save, ui.label.title, and the ui.notebook.* group) are left in English
in ja.json; replace each English string with the appropriate Japanese
translation (keep the keys unchanged), ensuring grammar/context fits UI labels
and descriptions, and run a quick scan for the same untranslated notebook/note
keys across other locale files (ko/es/pt/ru/etc.) to translate them consistently
as well.
In `@plugin/plugins/study_companion/static/style.css`:
- Around line 355-357: The .study-notebook-editor__meta rule only sets
grid-template-columns and is missing display: grid so the column layout is
ignored; update the .study-notebook-editor__meta selector to include display:
grid (and add a sensible gap if needed) so the four-column grid takes effect;
also ensure any responsive/nested overrides that set grid-template-columns
(e.g., switching to 1fr) keep or reapply display: grid and not rely on
.study-panel__state to provide the grid display.
In `@plugin/plugins/study_companion/store_notebook.py`:
- Around line 789-795: When normalized_filter == "specific" and
_nullable_text(notebook_id) returns None, the code currently omits any WHERE
clause and unintentionally returns all rows; change the branch in the function
handling normalized_filter to detect notebook_key is None and append a
restrictive clause such as f"{column_prefix}1=0" (or "1=0"/"FALSE") to clauses
(and do not append to params) so the query yields zero results for a missing
notebook_id; update the block that references notebook_key, notebook_id,
clauses, params, and column_prefix accordingly.
In `@plugin/plugins/study_companion/surfaces/note_search.tsx`:
- Around line 33-57: The runSearch function leaves busy=true when the user
clears the input because the early return on empty trimmed skips setBusy(false);
update runSearch so that when trimmed is empty it resets state and clears busy
(call setPayload({}), setStatus(''), and setBusy(false)) before returning; keep
existing abort checks in catch/finally intact so aborted requests still avoid
redundant state updates, and reference runSearch, setBusy, setPayload, and
setStatus when making the change.
---
Outside diff comments:
In `@plugin/plugins/study_companion/constants.py`:
- Around line 15-26: You added new constants LLM_OPERATION_EXPAND_NOTE and
LLM_OPERATION_SUMMARIZE_TO_NOTE but didn’t include them in the
SUPPORTED_LLM_OPERATIONS frozenset; update SUPPORTED_LLM_OPERATIONS to include
LLM_OPERATION_EXPAND_NOTE and LLM_OPERATION_SUMMARIZE_TO_NOTE so membership
checks (e.g., operation in SUPPORTED_LLM_OPERATIONS) will recognize these new
operations.
In `@plugin/plugins/study_companion/tutor_llm_agent.py`:
- Around line 480-524: The code assumes
config_manager.get_model_api_config(model_group) will always succeed for
model_group from model_group_override (e.g., "tutor"), but
utils.config_manager.get_model_api_config has no "tutor" mapping and will raise
ValueError causing a silent degradation; fix by catching that case where
api_config is requested for requested_model_group or for model_group selection
in tutor_llm_agent.py: wrap calls to
config_manager.get_model_api_config(requested_group) /
config_manager.get_model_api_config(model_group) in a try/except that catches
the unknown-model-type ValueError and then falls back to
config_manager.get_model_api_config("agent") (and set model_group="agent" and
call_type_group accordingly), or alternatively add a "tutor" entry to the
model_type_mapping inside utils.config_manager.get_model_api_config if the
intent is to support tutor—choose the approach consistent with product intent.
---
Nitpick comments:
In `@plugin/plugins/study_companion/entry_neko_commands.py`:
- Line 277: The return error messages use string concatenation in the
Err(SdkError("unknown command: " + cmd)) (and the similar case at line 282)
which is inconsistent with the file's f-string style (e.g., f"unclassified
command: {cmd}"); replace those concatenations with f-strings so the calls
become Err(SdkError(f"unknown command: {cmd}")) to match the rest of the file
and improve consistency for the error messages referencing cmd.
In `@plugin/plugins/study_companion/store_schema.py`:
- Around line 191-200: Wrap the FTS5 CREATE statement (the conn.execute call
that creates the notes_fts virtual table) in a try/except so initialization does
not abort on platforms where FTS5 is unavailable; on exception, log or record
the failure and fall back to the existing LIKE-based full‑text fallback path
(the code path that uses plain LIKE searches against the notes table) so
consumers can continue to operate without FTS5.
In `@plugin/plugins/study_companion/surfaces/note_editor.tsx`:
- Around line 45-69: The preview currently returns isolated <li> elements for
lines starting with '- ' (in function markdownPreview), producing orphaned list
items; update markdownPreview to group consecutive '- ' lines into a single <ul>
wrapper instead of returning individual <li> nodes: iterate through lines,
accumulate consecutive list-item strings into a buffer, flush that buffer as one
<ul key=...>{buffer.map((txt,i)=><li key=...>{txt}</li>)}</ul> when a non-list
line or end is reached, and handle other line types (headers, blockquote, blank,
paragraph) as before; ensure unique keys (using index and slice) are applied to
the generated <ul> and <li> elements.
In `@plugin/plugins/study_companion/surfaces/notebook_panel.tsx`:
- Around line 115-123: The effect watching query currently calls refresh
immediately so every keystroke triggers parallel fetches for study_notebook_list
and study_note_list; debounce the query before invoking refresh (create a
debouncedQuery via a useDebounce hook or an internal setTimeout/clearTimeout in
a useEffect) and change the dependency of the refresh-calling effect to
[selectedNotebookId, debouncedQuery]; also avoid refetching the notebook list on
query changes by splitting refresh into two callers or adding a flag so that
refresh only fetches study_note_list when invoked due to debouncedQuery, while
notebook fetching is done once on mount or after notebook-create (update places
referencing refresh, study_notebook_list, study_note_list, selectedNotebookId,
and query accordingly).
In `@plugin/plugins/study_companion/surfaces/study_panel.tsx`:
- Around line 177-197: renderMathSpans mutates the DOM nodes KaTeX renders, but
MathReply uses index-based keys like key={`math-${index}`} causing React to
reuse/mismatch spans; change the key on math spans in the MathReply component to
derive from the math content (e.g., part.value or a stable hash of it) instead
of the loop index so React will recreate the span when its math changes,
reducing reuse of externally-mutated DOM; update references where
key={`math-${index}`} is used to use key={part.value} (or a stable
content-derived key) and keep renderMathSpans and its safety checks unchanged.
In `@plugin/plugins/study_companion/tutor_llm_agent_notebook.py`:
- Around line 198-203: The fallback expand message in _fallback_expand_note is
hardcoded to Chinese; update it to be localized (mirror how summarize_to_note
uses _localized_note_headings) by accepting a language parameter or reading
self._config.language from expand_note and selecting a message map (e.g., using
the existing study_i18n_t pattern or a small dict keyed by normalized language
codes with an English default); ensure expand_note passes the language into
_fallback_expand_note (or let the method access self._config.language) so
non-Chinese users see the appropriate fallback text.
🪄 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: 94ef30ec-7eab-49e3-9a24-443edd6c8714
⛔ Files ignored due to path filters (62)
plugin/plugins/study_companion/static/fonts/KaTeX_AMS-Regular.ttfis excluded by!**/*.ttfplugin/plugins/study_companion/static/fonts/KaTeX_AMS-Regular.woffis excluded by!**/*.woffplugin/plugins/study_companion/static/fonts/KaTeX_AMS-Regular.woff2is excluded by!**/*.woff2plugin/plugins/study_companion/static/fonts/KaTeX_Caligraphic-Bold.ttfis excluded by!**/*.ttfplugin/plugins/study_companion/static/fonts/KaTeX_Caligraphic-Bold.woffis excluded by!**/*.woffplugin/plugins/study_companion/static/fonts/KaTeX_Caligraphic-Bold.woff2is excluded by!**/*.woff2plugin/plugins/study_companion/static/fonts/KaTeX_Caligraphic-Regular.ttfis excluded by!**/*.ttfplugin/plugins/study_companion/static/fonts/KaTeX_Caligraphic-Regular.woffis excluded by!**/*.woffplugin/plugins/study_companion/static/fonts/KaTeX_Caligraphic-Regular.woff2is excluded by!**/*.woff2plugin/plugins/study_companion/static/fonts/KaTeX_Fraktur-Bold.ttfis excluded by!**/*.ttfplugin/plugins/study_companion/static/fonts/KaTeX_Fraktur-Bold.woffis excluded by!**/*.woffplugin/plugins/study_companion/static/fonts/KaTeX_Fraktur-Bold.woff2is excluded by!**/*.woff2plugin/plugins/study_companion/static/fonts/KaTeX_Fraktur-Regular.ttfis excluded by!**/*.ttfplugin/plugins/study_companion/static/fonts/KaTeX_Fraktur-Regular.woffis excluded by!**/*.woffplugin/plugins/study_companion/static/fonts/KaTeX_Fraktur-Regular.woff2is excluded by!**/*.woff2plugin/plugins/study_companion/static/fonts/KaTeX_Main-Bold.ttfis excluded by!**/*.ttfplugin/plugins/study_companion/static/fonts/KaTeX_Main-Bold.woffis excluded by!**/*.woffplugin/plugins/study_companion/static/fonts/KaTeX_Main-Bold.woff2is excluded by!**/*.woff2plugin/plugins/study_companion/static/fonts/KaTeX_Main-BoldItalic.ttfis excluded by!**/*.ttfplugin/plugins/study_companion/static/fonts/KaTeX_Main-BoldItalic.woffis excluded by!**/*.woffplugin/plugins/study_companion/static/fonts/KaTeX_Main-BoldItalic.woff2is excluded by!**/*.woff2plugin/plugins/study_companion/static/fonts/KaTeX_Main-Italic.ttfis excluded by!**/*.ttfplugin/plugins/study_companion/static/fonts/KaTeX_Main-Italic.woffis excluded by!**/*.woffplugin/plugins/study_companion/static/fonts/KaTeX_Main-Italic.woff2is excluded by!**/*.woff2plugin/plugins/study_companion/static/fonts/KaTeX_Main-Regular.ttfis excluded by!**/*.ttfplugin/plugins/study_companion/static/fonts/KaTeX_Main-Regular.woffis excluded by!**/*.woffplugin/plugins/study_companion/static/fonts/KaTeX_Main-Regular.woff2is excluded by!**/*.woff2plugin/plugins/study_companion/static/fonts/KaTeX_Math-BoldItalic.ttfis excluded by!**/*.ttfplugin/plugins/study_companion/static/fonts/KaTeX_Math-BoldItalic.woffis excluded by!**/*.woffplugin/plugins/study_companion/static/fonts/KaTeX_Math-BoldItalic.woff2is excluded by!**/*.woff2plugin/plugins/study_companion/static/fonts/KaTeX_Math-Italic.ttfis excluded by!**/*.ttfplugin/plugins/study_companion/static/fonts/KaTeX_Math-Italic.woffis excluded by!**/*.woffplugin/plugins/study_companion/static/fonts/KaTeX_Math-Italic.woff2is excluded by!**/*.woff2plugin/plugins/study_companion/static/fonts/KaTeX_SansSerif-Bold.ttfis excluded by!**/*.ttfplugin/plugins/study_companion/static/fonts/KaTeX_SansSerif-Bold.woffis excluded by!**/*.woffplugin/plugins/study_companion/static/fonts/KaTeX_SansSerif-Bold.woff2is excluded by!**/*.woff2plugin/plugins/study_companion/static/fonts/KaTeX_SansSerif-Italic.ttfis excluded by!**/*.ttfplugin/plugins/study_companion/static/fonts/KaTeX_SansSerif-Italic.woffis excluded by!**/*.woffplugin/plugins/study_companion/static/fonts/KaTeX_SansSerif-Italic.woff2is excluded by!**/*.woff2plugin/plugins/study_companion/static/fonts/KaTeX_SansSerif-Regular.ttfis excluded by!**/*.ttfplugin/plugins/study_companion/static/fonts/KaTeX_SansSerif-Regular.woffis excluded by!**/*.woffplugin/plugins/study_companion/static/fonts/KaTeX_SansSerif-Regular.woff2is excluded by!**/*.woff2plugin/plugins/study_companion/static/fonts/KaTeX_Script-Regular.ttfis excluded by!**/*.ttfplugin/plugins/study_companion/static/fonts/KaTeX_Script-Regular.woffis excluded by!**/*.woffplugin/plugins/study_companion/static/fonts/KaTeX_Script-Regular.woff2is excluded by!**/*.woff2plugin/plugins/study_companion/static/fonts/KaTeX_Size1-Regular.ttfis excluded by!**/*.ttfplugin/plugins/study_companion/static/fonts/KaTeX_Size1-Regular.woffis excluded by!**/*.woffplugin/plugins/study_companion/static/fonts/KaTeX_Size1-Regular.woff2is excluded by!**/*.woff2plugin/plugins/study_companion/static/fonts/KaTeX_Size2-Regular.ttfis excluded by!**/*.ttfplugin/plugins/study_companion/static/fonts/KaTeX_Size2-Regular.woffis excluded by!**/*.woffplugin/plugins/study_companion/static/fonts/KaTeX_Size2-Regular.woff2is excluded by!**/*.woff2plugin/plugins/study_companion/static/fonts/KaTeX_Size3-Regular.ttfis excluded by!**/*.ttfplugin/plugins/study_companion/static/fonts/KaTeX_Size3-Regular.woffis excluded by!**/*.woffplugin/plugins/study_companion/static/fonts/KaTeX_Size3-Regular.woff2is excluded by!**/*.woff2plugin/plugins/study_companion/static/fonts/KaTeX_Size4-Regular.ttfis excluded by!**/*.ttfplugin/plugins/study_companion/static/fonts/KaTeX_Size4-Regular.woffis excluded by!**/*.woffplugin/plugins/study_companion/static/fonts/KaTeX_Size4-Regular.woff2is excluded by!**/*.woff2plugin/plugins/study_companion/static/fonts/KaTeX_Typewriter-Regular.ttfis excluded by!**/*.ttfplugin/plugins/study_companion/static/fonts/KaTeX_Typewriter-Regular.woffis excluded by!**/*.woffplugin/plugins/study_companion/static/fonts/KaTeX_Typewriter-Regular.woff2is excluded by!**/*.woff2plugin/plugins/study_companion/static/katex.min.jsis excluded by!**/*.min.jsuv.lockis excluded by!**/*.lock
📒 Files selected for processing (47)
plugin/plugins/study_companion/__init__.pyplugin/plugins/study_companion/constants.pyplugin/plugins/study_companion/doc_exporter.pyplugin/plugins/study_companion/entry_export_support.pyplugin/plugins/study_companion/entry_knowledge_entries.pyplugin/plugins/study_companion/entry_neko_commands.pyplugin/plugins/study_companion/entry_notebook.pyplugin/plugins/study_companion/i18n/en.jsonplugin/plugins/study_companion/i18n/es.jsonplugin/plugins/study_companion/i18n/ja.jsonplugin/plugins/study_companion/i18n/ko.jsonplugin/plugins/study_companion/i18n/pt.jsonplugin/plugins/study_companion/i18n/ru.jsonplugin/plugins/study_companion/i18n/zh-CN.jsonplugin/plugins/study_companion/i18n/zh-TW.jsonplugin/plugins/study_companion/models.pyplugin/plugins/study_companion/onboarding.mdplugin/plugins/study_companion/plugin.tomlplugin/plugins/study_companion/screen_classifier.pyplugin/plugins/study_companion/service.pyplugin/plugins/study_companion/static/index.htmlplugin/plugins/study_companion/static/katex-render.jsplugin/plugins/study_companion/static/katex.min.cssplugin/plugins/study_companion/static/main.jsplugin/plugins/study_companion/static/math-parser.jsplugin/plugins/study_companion/static/style.cssplugin/plugins/study_companion/store_notebook.pyplugin/plugins/study_companion/store_schema.pyplugin/plugins/study_companion/study_ocr_pipeline.pyplugin/plugins/study_companion/surfaces/memory_shared.tsplugin/plugins/study_companion/surfaces/note_card.tsxplugin/plugins/study_companion/surfaces/note_editor.tsxplugin/plugins/study_companion/surfaces/note_search.tsxplugin/plugins/study_companion/surfaces/notebook_panel.tsxplugin/plugins/study_companion/surfaces/study_panel.tsxplugin/plugins/study_companion/tutor_llm_agent.pyplugin/plugins/study_companion/tutor_llm_agent_notebook.pyplugin/plugins/study_companion/voice_contracts.pyplugin/plugins/study_companion/voice_filter.pyplugin/tests/unit/plugins/test_study_companion_neko_commands.pyplugin/tests/unit/plugins/test_study_companion_phase9_ux.pyplugin/tests/unit/plugins/test_study_companion_voice_bridge.pyplugin/tests/unit/plugins/test_study_companion_voice_contracts.pyplugin/tests/unit/plugins/test_study_companion_voice_filter.pyplugin/tests/unit/plugins/test_study_companion_voice_scenarios.pyplugin/tests/unit/plugins/test_study_notebook.pypyproject.toml
💤 Files with no reviewable changes (1)
- plugin/plugins/study_companion/screen_classifier.py
There was a problem hiding this comment.
🧹 Nitpick comments (1)
plugin/server/application/plugins/dispatch_service.py (1)
278-283: ⚡ Quick win超时分支
%s不会丢占位符,只是和同文件风格不一致喵
这行的%s占位符会沿用 stdlib logging 的 %-格式化去填入plugin_id / event_type / event_id(PluginLoggerAdapter只会对包含{}的消息做 brace 适配),不会出现把占位符原样打印出来的情况喵。若想和同文件其它日志统一,也可以把它改成{}风格喵。🐾 建议改法喵
logger.warning( - "trigger_custom_event_subscribers handler timed out: plugin_id=%s event_type=%s event_id=%s", + "trigger_custom_event_subscribers handler timed out: plugin_id={}, event_type={}, event_id={}", plugin_id, event_type, handler_event_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 `@plugin/server/application/plugins/dispatch_service.py` around lines 278 - 283, The warning uses %-style placeholders but the codebase (and PluginLoggerAdapter) prefers brace-style logging; update the logger.warning call that logs "trigger_custom_event_subscribers handler timed out" to use "{}" placeholders and pass plugin_id, event_type, handler_event_id as arguments (keep the same variables) so the PluginLoggerAdapter will perform brace formatting consistently; locate the call to logger.warning in dispatch_service.py (the timeout branch for trigger_custom_event_subscribers) and replace the format string only, leaving the argument order and variables unchanged.
🤖 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 `@plugin/server/application/plugins/dispatch_service.py`:
- Around line 278-283: The warning uses %-style placeholders but the codebase
(and PluginLoggerAdapter) prefers brace-style logging; update the logger.warning
call that logs "trigger_custom_event_subscribers handler timed out" to use "{}"
placeholders and pass plugin_id, event_type, handler_event_id as arguments (keep
the same variables) so the PluginLoggerAdapter will perform brace formatting
consistently; locate the call to logger.warning in dispatch_service.py (the
timeout branch for trigger_custom_event_subscribers) and replace the format
string only, leaving the argument order and variables unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 6df881d6-ecef-47b3-a797-5242752626bb
📒 Files selected for processing (3)
plugin/plugins/study_companion/voice_contracts.pyplugin/server/application/plugins/dispatch_service.pyplugin/server/application/plugins/voice_contracts.py
🚧 Files skipped from review as they are similar to previous changes (1)
- plugin/plugins/study_companion/voice_contracts.py
There was a problem hiding this comment.
🧹 Nitpick comments (2)
plugin/plugins/study_companion/study_ocr_pipeline.py (1)
297-312: 💤 Low value这是 Average Hash 而不是 Perceptual Hash 喵~
函数名叫
_image_phash但实现的其实是 Average Hash(aHash)算法喵。真正的 pHash(Perceptual Hash)会使用 DCT 变换喵。不过对于检测"内容是否变化"这个用途,aHash 已经足够了,而且计算更快喵~如果想让命名更准确,可以考虑改名为
_image_ahash或在注释里说明,但这不是必须的改动喵~♻️ 可选的命名改进喵
`@staticmethod` - def _image_phash(image: Any) -> str: + def _image_ahash(image: Any) -> str: + """Compute a simple average hash (8x8 grayscale, 64-bit).""" if image is None or not hasattr(image, "convert"): return ""然后更新调用点:
- thumbnail_phash = self._image_phash(image) + thumbnail_phash = self._image_ahash(image)🤖 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 `@plugin/plugins/study_companion/study_ocr_pipeline.py` around lines 297 - 312, The method _image_phash implements an Average Hash (aHash) not a Perceptual Hash (pHash); either rename the function to _image_ahash (and update all call sites referencing _image_phash) or keep the name but add a clear comment/docstring stating it uses aHash (and update any documentation/tests accordingly) so callers and reviewers are not misled by the name; search for usages of _image_phash and change them to _image_ahash if you choose the rename to keep references consistent.plugin/plugins/study_companion/__init__.py (1)
660-677: 💤 Low valuecollector 启动失败后 awareness loop 仍会继续运行喵
当
os_signals_enabled=True但 collector 启动失败时,异常被捕获且_collector设为None,但循环会继续运行。这是符合预期的降级行为喵~ 不过本喵建议在日志里明确说明将以无系统信号模式继续运行,这样用户更容易理解发生了什么喵!♻️ 可选的日志改进喵
except Exception: self._collector = None self.logger.warning( - "study awareness collector startup failed", + "study awareness collector startup failed; continuing without os signals", exc_info=True, )🤖 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 `@plugin/plugins/study_companion/__init__.py` around lines 660 - 677, When collector startup in _run_awareness_loop fails (os_signals_enabled True) the code currently sets self._collector = None and logs a generic warning; update the warning in the except block to clearly state that the collector failed to start and the loop will continue in "no system signals" / degraded mode (keep exc_info=True so the exception is logged), so readers know _run_awareness_loop will fall back to polling via awareness_tick with self._collector == None; reference symbols: _run_awareness_loop, _collector, _cfg.awareness.os_signals_enabled, awareness_tick, logger.warning.
🤖 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 `@plugin/plugins/study_companion/__init__.py`:
- Around line 660-677: When collector startup in _run_awareness_loop fails
(os_signals_enabled True) the code currently sets self._collector = None and
logs a generic warning; update the warning in the except block to clearly state
that the collector failed to start and the loop will continue in "no system
signals" / degraded mode (keep exc_info=True so the exception is logged), so
readers know _run_awareness_loop will fall back to polling via awareness_tick
with self._collector == None; reference symbols: _run_awareness_loop,
_collector, _cfg.awareness.os_signals_enabled, awareness_tick, logger.warning.
In `@plugin/plugins/study_companion/study_ocr_pipeline.py`:
- Around line 297-312: The method _image_phash implements an Average Hash
(aHash) not a Perceptual Hash (pHash); either rename the function to
_image_ahash (and update all call sites referencing _image_phash) or keep the
name but add a clear comment/docstring stating it uses aHash (and update any
documentation/tests accordingly) so callers and reviewers are not misled by the
name; search for usages of _image_phash and change them to _image_ahash if you
choose the rename to keep references consistent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 97f05ea0-2e9e-450d-b3c7-ac8f52e962ba
📒 Files selected for processing (11)
plugin/plugins/study_companion/__init__.pyplugin/plugins/study_companion/entry_notebook.pyplugin/plugins/study_companion/i18n/ja.jsonplugin/plugins/study_companion/models.pyplugin/plugins/study_companion/plugin.tomlplugin/plugins/study_companion/screen_classifier.pyplugin/plugins/study_companion/static/style.cssplugin/plugins/study_companion/store_notebook.pyplugin/plugins/study_companion/study_ocr_pipeline.pyplugin/plugins/study_companion/surfaces/note_search.tsxplugin/tests/unit/plugins/test_study_notebook.py
🚧 Files skipped from review as they are similar to previous changes (6)
- plugin/plugins/study_companion/i18n/ja.json
- plugin/plugins/study_companion/surfaces/note_search.tsx
- plugin/tests/unit/plugins/test_study_notebook.py
- plugin/plugins/study_companion/static/style.css
- plugin/plugins/study_companion/entry_notebook.py
- plugin/plugins/study_companion/store_notebook.py
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)
plugin/plugins/study_companion/tutor_llm_agent.py (1)
517-528:⚠️ Potential issue | 🟠 Major | ⚡ Quick win给
vision分组也走同一套未知分组回退逻辑喵。这里直接调用
get_model_api_config("vision"),如果实例没有配置/不支持vision分组,会立刻抛ValueError,整次带图请求直接失败,后面的 agent 回退和图片剥离都进不去喵。你前面刚加的_get_model_api_config()已经处理了未知分组回退,这里也该复用一下喵。🐾 建议改法喵
if has_image: - vision_config = config_manager.get_model_api_config("vision") + vision_config, resolved_group = _get_model_api_config("vision") vision_base_url = str(vision_config.get("base_url") or "").strip() vision_model = str(vision_config.get("model") or "").strip() if vision_base_url and vision_model: api_config = vision_config - model_group = "vision" - call_type_group = "vision" + model_group = resolved_group + call_type_group = resolved_group else: - api_config = config_manager.get_model_api_config("agent") - model_group = "agent" - call_type_group = "agent" + api_config, model_group = _get_model_api_config("agent") + call_type_group = model_group🤖 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 `@plugin/plugins/study_companion/tutor_llm_agent.py` around lines 517 - 528, The code calls config_manager.get_model_api_config("vision") which throws for unknown groups; replace that call with the already-safe helper _get_model_api_config("vision") so the vision branch reuses the unknown-group fallback logic — obtain vision_config via _get_model_api_config("vision"), then keep the existing checks/assignments for vision_base_url, vision_model and set api_config, model_group, call_type_group accordingly so image requests can gracefully fall back to the agent config when vision is not configured.
🤖 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 `@plugin/plugins/study_companion/tutor_llm_agent.py`:
- Around line 517-528: The code calls
config_manager.get_model_api_config("vision") which throws for unknown groups;
replace that call with the already-safe helper _get_model_api_config("vision")
so the vision branch reuses the unknown-group fallback logic — obtain
vision_config via _get_model_api_config("vision"), then keep the existing
checks/assignments for vision_base_url, vision_model and set api_config,
model_group, call_type_group accordingly so image requests can gracefully fall
back to the agent config when vision is not configured.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 1c2c4b98-aead-446d-b4c8-90a948b6450e
📒 Files selected for processing (11)
plugin/plugins/study_companion/constants.pyplugin/plugins/study_companion/store_schema.pyplugin/plugins/study_companion/surfaces/note_editor.tsxplugin/plugins/study_companion/surfaces/notebook_panel.tsxplugin/plugins/study_companion/surfaces/study_panel.tsxplugin/plugins/study_companion/tutor_llm_agent.pyplugin/plugins/study_companion/tutor_llm_agent_notebook.pyplugin/tests/unit/plugins/test_study_companion.pyplugin/tests/unit/plugins/test_study_companion_llm_prompts.pyplugin/tests/unit/plugins/test_study_companion_phase9_ux.pyplugin/tests/unit/plugins/test_study_notebook.py
🚧 Files skipped from review as they are similar to previous changes (7)
- plugin/plugins/study_companion/surfaces/notebook_panel.tsx
- plugin/tests/unit/plugins/test_study_companion_phase9_ux.py
- plugin/plugins/study_companion/surfaces/study_panel.tsx
- plugin/plugins/study_companion/surfaces/note_editor.tsx
- plugin/plugins/study_companion/store_schema.py
- plugin/plugins/study_companion/tutor_llm_agent_notebook.py
- plugin/tests/unit/plugins/test_study_notebook.py
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0544fa2ad2
ℹ️ 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".
| ocr_text, _boxes = self._normalize_ocr_output(raw) | ||
| except Exception as exc: | ||
| return LightweightSnapshot( | ||
| status="ocr_failed", |
There was a problem hiding this comment.
Preserve title-only awareness when OCR fails
When awareness is configured with classify_mode ocr_text or both, any OCR backend exception now returns a LightweightSnapshot with status ocr_failed; awareness_tick treats anything outside {"ok", "empty"} as no usable snapshot and returns without adding even the title/app activity. In environments where OCR is missing or temporarily errors, this disables awareness tracking entirely despite the window title and app type already being available here, whereas the previous path kept an ok title-based snapshot and only omitted OCR classification.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 764ca6c5be
ℹ️ 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".
| @custom_event( | ||
| event_type=VOICE_TRANSCRIPT_EVENT_TYPE, | ||
| id=VOICE_TRANSCRIPT_EVENT_ID, | ||
| name="Handle study voice transcript", | ||
| description="Filter realtime study voice transcripts and return a voice-session action.", | ||
| input_schema={ | ||
| "type": "object", | ||
| "properties": { | ||
| "transcript": {"type": "string"}, | ||
| "lanlan_name": {"type": "string"}, | ||
| "metadata": {"type": "object"}, | ||
| }, | ||
| "required": ["transcript"], | ||
| }, | ||
| trigger_method="manual", |
There was a problem hiding this comment.
Wire the voice handler into the live transcript path
Registering this custom_event handler does not by itself put study companion on the realtime voice path: I checked the live transcript entry points (main_routers/game_router.py::route_external_voice_transcript and its audio branch) and the new subscriber dispatcher, and there is no call site for trigger_custom_event_subscribers/voice arbitration outside tests. In normal voice sessions, transcripts therefore continue through the existing game/ordinary routing without ever invoking this handler, so the new cancel/prime-context behavior is unreachable unless another plugin manually calls it.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c94e87506c
ℹ️ 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".
| "request_id": request_id, | ||
| "voice_session_id": request_id, | ||
| "source": "realtime_stt", |
There was a problem hiding this comment.
Keep voice session key stable across transcripts
When realtime voice is active, handle_input_transcript creates a fresh realtime-stt-<uuid> for every transcript and this code sends it as voice_session_id; the study plugin keys its NAME_WINDOW_SECONDS follow-up state from voice_session_id before falling back to the character name. In a live session, saying “Yui …” and then a short follow-up within three seconds therefore lands in a different session key, so the intended name_window relay is missed and the follow-up can be dropped as too short or OCR overlap. Use a stable conversation/session id here, or keep the per-turn UUID only as request_id so the plugin can fall back to a stable key.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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.py`:
- Around line 2205-2239: The current logic calls
_dispatch_voice_transcript_subscribers(...) before _takeover_input_dispatcher
and returns early if subscribers handled the transcript, which prevents the
takeover handler from ever receiving STT; change the flow so the takeover path
runs first when self._takeover_input_dispatcher is set (invoke
self._takeover_input_dispatcher with lanlan_name, transcript_text and request_id
and act on its boolean result), and only if takeover did not handle the
transcript then call _dispatch_voice_transcript_subscribers(...); also preserve
the voice_rms_recorded/_activity_tracker semantics (ensure on_voice_rms() is
invoked once before takeover or subscriber handling as before).
- Around line 152-177: The lazy imports in
_get_voice_transcript_dispatch_service and
_dispatch_voice_transcript_subscribers currently pull
plugin.server.application.plugins.* (PluginDispatchService,
VOICE_TRANSCRIPT_EVENT_ID, VOICE_TRANSCRIPT_EVENT_TYPE,
VOICE_TRANSCRIPT_ACTION_PRIME_CONTEXT, VOICE_TRANSCRIPT_ACTION_CANCEL_RESPONSE,
arbitrate_voice_transcript_results) making core depend on the plugin layer; fix
by either (A) moving the voice_transcript contract constants and the
arbitrate_voice_transcript_results helper into a lower shared/server boundary
module that main_logic can import, and update both
_get_voice_transcript_dispatch_service and
_dispatch_voice_transcript_subscribers to import those shared symbols instead,
or (B) change _get_voice_transcript_dispatch_service to accept an injected
dispatcher/adapter instance (or provider) from the outer layer and have
_dispatch_voice_transcript_subscribers use that injected dispatcher and only
import lightweight shared contract types; ensure PluginDispatchService is
instantiated outside core and passed in, or that constants/arbitration logic are
relocated to the shared layer so main_logic no longer references
plugin.server.application.plugins.*.
In `@tests/unit/test_core_game_route_memory_contract.py`:
- Around line 395-420: The fake dispatch currently returns a prime_context
action but does not record or assert the dispatch call parameters, so update
_FakeDispatchService.trigger_custom_event_subscribers to capture and store the
incoming kwargs (e.g., in self.last_call_kwargs) and then in the test (after
calling the transcript manager) assert that the dispatch was invoked with the
expected event_type/event_id and args (matching the real dispatch signature used
by handle_input_transcript/handle_transcript); also assert that
_FakeVoiceSession.prime_context_calls contains the expected (text, skipped)
tuple; apply the same capture-and-assert pattern for the other similar case
around lines 422-433 to prevent silent misrouting.
🪄 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: 273b503e-050d-4034-88bb-56d67358aab6
📒 Files selected for processing (2)
main_logic/core.pytests/unit/test_core_game_route_memory_contract.py
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
main_logic/agent_event_bus.py (1)
327-411: 💤 Low value本喵觉得这段并发协调逻辑写得还不错呢喵~
_resolve_voice_bridge_seen_waiter作为共享 helper 抽出来复用很合理,loop.call_soon_threadsafe的异常处理也有兜底(RuntimeError 时清理 waiter 并 cancel)喵。notify_voice_bridge_result里在_resolve闭包内再次持锁验证 waiter 身份的做法是正确的防御式编程喵~不过本喵稍微有点小担心:
_voice_bridge_waiters_queued和_voice_bridge_waiters_resolving两个 set 的语义区分比较微妙,后续维护者可能需要花点时间理解"queued = 已入队但未开始 resolve"和"resolving = 正在 resolve 中"的差异喵。建议在模块顶部或者这几个变量旁边加一小段注释说明状态机流转,方便以后的猫娘们维护喵~🤖 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/agent_event_bus.py` around lines 327 - 411, The logic is correct but the semantics of the two state sets _voice_bridge_waiters_queued and _voice_bridge_waiters_resolving are subtle; add a short explanatory comment by their declarations (or at module top) that defines each set ("queued = request has been enqueued on the main loop but not yet being resolved" and "resolving = resolution has started/being processed"), describe typical state transitions (queued -> resolving -> removed) and note concurrency/lock expectations (access under _voice_bridge_waiters_lock), so future maintainers can quickly understand the state machine used by notify_voice_bridge_request_seen, mark_voice_bridge_result_queued, and notify_voice_bridge_result.
🤖 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 `@app/agent_server.py`:
- Around line 1760-1767: The realtime transcript handler currently awaits
_ensure_plugin_lifecycle_started() and blocks the voice path; instead, when
Modules.plugin_lifecycle_started is false, immediately return a noop indicating
warming up (e.g. use _voice_bridge_noop("plugin_warming_up")) rather than
awaiting _ensure_plugin_lifecycle_started(), or if you prefer a best-effort
start attempt use a very short timeout around _ensure_plugin_lifecycle_started()
and fall back to the noop on timeout; update the branch that sets
lifecycle_ready and the call sites that currently do await
_ensure_plugin_lifecycle_started() (refer to Modules.plugin_lifecycle_started,
_ensure_plugin_lifecycle_started, _voice_bridge_noop, and
_dispatch_voice_transcript_custom_event) so the realtime chain never blocks on
plugin cold-start.
- Around line 1654-1730: You duplicated the voice_transcript contract locally
which drifts from the shared contract; instead remove the local constants and
helpers (VOICE_TRANSCRIPT_CUSTOM_EVENT_TYPE, VOICE_TRANSCRIPT_CUSTOM_EVENT_ID,
_voice_bridge_noop, _voice_transcript_request_has_text,
_voice_transcript_custom_event_args) and import the canonical symbols and
helpers from plugin.server.application.plugins.voice_contracts (e.g. import
VOICE_TRANSCRIPT_CUSTOM_EVENT_TYPE, VOICE_TRANSCRIPT_CUSTOM_EVENT_ID, noop
helper and any request/arg helpers and arbitrate_voice_transcript_results), then
update _voice_bridge_action_from_dispatch_results to consume dispatch results
and return the exact payload produced/normalized by the shared helpers (so
priority/skipped and failures are preserved), and replace any local usages to
reference the imported constants/functions.
In `@main_logic/core.py`:
- Around line 2009-2022: The bridge currently returns the string
"cancel_response" even when the cancel_response side-effect was missing or
failed, which causes handle_input_transcript() to early-return and drop normal
bookkeeping; modify _dispatch_voice_transcript_bridge() so that it separates
performing the side-effect from claiming the transcript as consumed: only return
"cancel_response" (or set a consumed=true) when session_snapshot.cancel_response
existed, was callable, and completed without error (and _session_changed()
stayed false); if cancel_response is missing or raises an exception, log the
failure but fall back to returning a non-consumed result (e.g., None or
consumed=false) so handle_input_transcript() continues the normal transcript
flow (on_user_message, _session_turn_count increment,
_publish_user_utterance_to_plugin_bus, sync queue, and user_transcript push).
Ensure you reference and preserve the existing _session_changed() check and
exception handling around cancel_response.
---
Nitpick comments:
In `@main_logic/agent_event_bus.py`:
- Around line 327-411: The logic is correct but the semantics of the two state
sets _voice_bridge_waiters_queued and _voice_bridge_waiters_resolving are
subtle; add a short explanatory comment by their declarations (or at module top)
that defines each set ("queued = request has been enqueued on the main loop but
not yet being resolved" and "resolving = resolution has started/being
processed"), describe typical state transitions (queued -> resolving -> removed)
and note concurrency/lock expectations (access under
_voice_bridge_waiters_lock), so future maintainers can quickly understand the
state machine used by notify_voice_bridge_request_seen,
mark_voice_bridge_result_queued, and notify_voice_bridge_result.
🪄 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: 102f5e7e-17e7-43e4-b432-645aee704eec
📒 Files selected for processing (5)
app/agent_server.pyapp/main_server.pymain_logic/agent_event_bus.pymain_logic/core.pytests/unit/test_core_game_route_memory_contract.py
40e2938 to
cdce270
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cdce270a80
ℹ️ 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".
| if (!source || !source.includes('$') || !window.katex || typeof window.katex.renderToString !== 'function') { | ||
| return escapeHTML(source); |
There was a problem hiding this comment.
Restore backslash-delimited math rendering
When a tutor reply uses LaTeX delimiters like \(x^2\) or \[...\] without any $, this new guard returns escaped text before calling the shared parser, even though math-parser.js still supports those delimiters. This regresses the static study UI for a common math format that the previous renderer handled; include source.includes('\\(')/source.includes('\\[') in the fast-path check or let the parser decide.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (3)
app/agent_server.py (2)
1654-1744:⚠️ Potential issue | 🟠 Major别在这里再维护第二份
voice_transcript契约喵。共享的 event type / event id / 仲裁逻辑已经在服务端
voice_contracts.py里定义好了,study_companion 侧也是按那套常量注册订阅的;这里继续手写VOICE_TRANSCRIPT_CUSTOM_EVENT_*,再自己重组一遍dispatch_results,会把跨边界契约拆成两份真相源,后面共享常量或结果 schema 一变,这里就会静默退化成no_subscribers或丢掉共享归一化结果喵。建议直接复用共享常量,并把原始dispatch_results直接交给共享仲裁函数处理喵。🐾 建议收敛到共享契约喵
+from plugin.server.application.plugins.voice_contracts import ( + VOICE_TRANSCRIPT_EVENT_TYPE as VOICE_TRANSCRIPT_CUSTOM_EVENT_TYPE, + VOICE_TRANSCRIPT_EVENT_ID as VOICE_TRANSCRIPT_CUSTOM_EVENT_ID, + arbitrate_voice_transcript_results, +) - -VOICE_TRANSCRIPT_CUSTOM_EVENT_TYPE = "voice_transcript" -VOICE_TRANSCRIPT_CUSTOM_EVENT_ID = "handle_transcript" VOICE_TRANSCRIPT_CUSTOM_EVENT_TIMEOUT_SECONDS = 1.0 ... def _voice_bridge_action_from_dispatch_results(dispatch_results: object) -> Dict[str, Any]: - if not isinstance(dispatch_results, list) or not dispatch_results: - return _voice_bridge_noop("no_subscribers") - - from plugin.server.application.plugins.voice_contracts import ( - arbitrate_voice_transcript_results, - ) - - arbitration_items: list[dict[str, object]] = [] - failure_count = 0 - for item in dispatch_results: - ... - if not arbitration_items: - return _voice_bridge_noop("no_handler_result", failures=failure_count) - payload = arbitrate_voice_transcript_results(arbitration_items) - if failure_count: - ... - return payload + return arbitrate_voice_transcript_results(dispatch_results)🤖 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 `@app/agent_server.py` around lines 1654 - 1744, This duplicates the voice_transcript contract (VOICE_TRANSCRIPT_CUSTOM_EVENT_TYPE/ID and local arbitration in _voice_bridge_action_from_dispatch_results) instead of reusing the shared contract; update _dispatch_voice_transcript_custom_event to import and use the shared constants and pass the raw dispatch_results into the shared arbiter (arbitrate_voice_transcript_results) rather than rebuilding items, remove or delegate _voice_transcript_custom_event_args to the shared arg/schema helper if one exists, and delete the local VOICE_TRANSCRIPT_CUSTOM_EVENT_* constants and the custom reconstruction logic in _voice_bridge_action_from_dispatch_results so the code delegates to the shared contract functions (refer to _dispatch_voice_transcript_custom_event, _voice_transcript_custom_event_args, _voice_bridge_action_from_dispatch_results, and arbitrate_voice_transcript_results to locate the changes).
1760-1767:⚠️ Potential issue | 🟠 Major实时转写路径不要在这里等插件冷启动喵。
这里把
voice_transcript_request卡在await _ensure_plugin_lifecycle_started()上了,而 1 秒超时预算只包住后面的 subscriber dispatch,不包生命周期启动本身喵。第一次 transcript 很容易等到插件起来了才拿到结果,但那时 cancel/prime 的时机已经过了,整条语音桥接就会变成过期决策喵。这个分支更稳的做法是直接回noop("plugin_warming_up"),把冷启动留给已有的 enable/restore 后台流程喵。😼 一个更稳的收口方式喵
else: - lifecycle_ready = bool(Modules.plugin_lifecycle_started) - if not lifecycle_ready: - lifecycle_ready = await _ensure_plugin_lifecycle_started() - - if not lifecycle_ready: - result = _voice_bridge_noop("plugin_lifecycle_start_failed") - else: - result = await _dispatch_voice_transcript_custom_event(event) + if not Modules.plugin_lifecycle_started: + result = _voice_bridge_noop("plugin_warming_up") + else: + result = await _dispatch_voice_transcript_custom_event(event)🤖 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 `@app/agent_server.py` around lines 1760 - 1767, The real-time transcript path must not block on plugin cold-start; replace the await of _ensure_plugin_lifecycle_started() in the branch that checks Modules.plugin_lifecycle_started so that if Modules.plugin_lifecycle_started is False you immediately return result = _voice_bridge_noop("plugin_warming_up") instead of awaiting _ensure_plugin_lifecycle_started(), otherwise proceed to await _dispatch_voice_transcript_custom_event(event); update the logic around lifecycle_ready, _ensure_plugin_lifecycle_started, _voice_bridge_noop and _dispatch_voice_transcript_custom_event in app/agent_server.py accordingly.main_logic/core.py (1)
2009-2022:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
cancel_response失败时别把这条 transcript 也一起吞掉喵。这里即使
cancel_response不存在、不可调用,或者 await 时报错,末尾也还是return action喵。handle_input_transcript()在 Line 2261 收到"cancel_response"后会直接早退,所以这句用户转写后面的on_user_message、turn 计数、_publish_user_utterance_to_plugin_bus()和前端user_transcript都会被整条跳过喵。只有真的完成了 cancel side effect,才应该返回"cancel_response";失败时应该回落成普通 transcript 流程喵。🛠️ 一个局部修正方向喵
if action == "cancel_response": cancel_response = getattr(session_snapshot, "cancel_response", None) - if callable(cancel_response): - try: - if _session_changed(): - return "" - await cancel_response() - logger.debug("[%s] voice bridge cancelled current response", self.lanlan_name) - except asyncio.CancelledError: - raise - except Exception as exc: - logger.debug("[%s] voice bridge cancel skipped/failed: %s", self.lanlan_name, exc) - return action + if not callable(cancel_response): + return "" + try: + if _session_changed(): + return "" + await cancel_response() + if _session_changed(): + return "" + logger.debug("[%s] voice bridge cancelled current response", self.lanlan_name) + return action + except asyncio.CancelledError: + raise + except Exception as exc: + logger.debug("[%s] voice bridge cancel skipped/failed: %s", self.lanlan_name, exc) + return ""Also applies to: 2256-2262
🤖 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.py` around lines 2009 - 2022, The current branch returns action == "cancel_response" regardless of whether session_snapshot.cancel_response existed, was callable, or succeeded, causing handle_input_transcript() to short-circuit and skip normal transcript handling; change the logic in the cancel_response handling (around the action variable and the block using session_snapshot.cancel_response, _session_changed(), and await cancel_response()) so that you only return "cancel_response" after the cancel_response callable was found and awaited successfully (no exception and _session_changed() false); on any failure (missing/non-callable cancel_response, _session_changed() true, or exception during await) fall back to the normal transcript flow by NOT returning action and allowing subsequent processing (on_user_message, turn counting, _publish_user_utterance_to_plugin_bus, and user_transcript) to proceed. Ensure you reference session_snapshot, cancel_response, _session_changed(), and the surrounding return action to implement this conditional return behavior.
🧹 Nitpick comments (1)
plugin/plugins/study_companion/models.py (1)
242-248: 💤 Low value
_clamp_int和_clamp_int_or_default功能重复了喵~第 143-153 行已经有一个
_clamp_int_or_default做同样的事情,这里又定义了一个几乎一样的_clamp_int,有点多余呢。可以考虑统一用一个就好了喵~🤖 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 `@plugin/plugins/study_companion/models.py` around lines 242 - 248, There are two duplicate helpers (_clamp_int and _clamp_int_or_default) implementing the same logic; remove the redundant one and consolidate to a single implementation (prefer keeping _clamp_int_or_default), then replace all usages of _clamp_int with _clamp_int_or_default (or vice versa) so callers use the single function and the signatures match (minimum, maximum, default, and value handling); ensure tests/type hints still compile after removing the duplicate.
🤖 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.py`:
- Around line 1977-2007: The code only checks session identity (self.session is
session_snapshot) to ignore stale bridge results but multiple concurrent STT
bridge requests can run for the same session; record and compare the per-request
realtime_voice_request_id to avoid applying late results to a newer turn: add an
instance field (e.g. self._latest_voice_transcript_request_id) initialized in
__init__, set it immediately when handle_input_transcript generates
realtime_voice_request_id, and then before and after awaiting
publish_voice_transcript_request_reliably (and before calling cancel_response()
/ prime_context()) verify the awaited request_id matches
self._latest_voice_transcript_request_id and bail out if not; update or add
similar checks around the blocks referenced by
publish_voice_transcript_request_reliably, _session_changed(),
cancel_response(), and prime_context().
In `@plugin/plugins/study_companion/static/index.html`:
- Line 12: The ARIA localization attribute used (data-i18n-aria-label) is not
recognized by our i18n injection logic, so replace every occurrence of
data-i18n-aria-label="..." with the project-standard data-i18n-aria="..."
(preserving the same translation key) for the elements mentioned (e.g., the
<section class="study-toolbar"> and the other elements at the noted locations)
so that i18n-i18next.js (which uses
document.querySelectorAll('[data-i18n-aria]')) will correctly write the
localized aria-label; keep the existing aria-label values as safe fallbacks if
present.
---
Duplicate comments:
In `@app/agent_server.py`:
- Around line 1654-1744: This duplicates the voice_transcript contract
(VOICE_TRANSCRIPT_CUSTOM_EVENT_TYPE/ID and local arbitration in
_voice_bridge_action_from_dispatch_results) instead of reusing the shared
contract; update _dispatch_voice_transcript_custom_event to import and use the
shared constants and pass the raw dispatch_results into the shared arbiter
(arbitrate_voice_transcript_results) rather than rebuilding items, remove or
delegate _voice_transcript_custom_event_args to the shared arg/schema helper if
one exists, and delete the local VOICE_TRANSCRIPT_CUSTOM_EVENT_* constants and
the custom reconstruction logic in _voice_bridge_action_from_dispatch_results so
the code delegates to the shared contract functions (refer to
_dispatch_voice_transcript_custom_event, _voice_transcript_custom_event_args,
_voice_bridge_action_from_dispatch_results, and
arbitrate_voice_transcript_results to locate the changes).
- Around line 1760-1767: The real-time transcript path must not block on plugin
cold-start; replace the await of _ensure_plugin_lifecycle_started() in the
branch that checks Modules.plugin_lifecycle_started so that if
Modules.plugin_lifecycle_started is False you immediately return result =
_voice_bridge_noop("plugin_warming_up") instead of awaiting
_ensure_plugin_lifecycle_started(), otherwise proceed to await
_dispatch_voice_transcript_custom_event(event); update the logic around
lifecycle_ready, _ensure_plugin_lifecycle_started, _voice_bridge_noop and
_dispatch_voice_transcript_custom_event in app/agent_server.py accordingly.
In `@main_logic/core.py`:
- Around line 2009-2022: The current branch returns action == "cancel_response"
regardless of whether session_snapshot.cancel_response existed, was callable, or
succeeded, causing handle_input_transcript() to short-circuit and skip normal
transcript handling; change the logic in the cancel_response handling (around
the action variable and the block using session_snapshot.cancel_response,
_session_changed(), and await cancel_response()) so that you only return
"cancel_response" after the cancel_response callable was found and awaited
successfully (no exception and _session_changed() false); on any failure
(missing/non-callable cancel_response, _session_changed() true, or exception
during await) fall back to the normal transcript flow by NOT returning action
and allowing subsequent processing (on_user_message, turn counting,
_publish_user_utterance_to_plugin_bus, and user_transcript) to proceed. Ensure
you reference session_snapshot, cancel_response, _session_changed(), and the
surrounding return action to implement this conditional return behavior.
---
Nitpick comments:
In `@plugin/plugins/study_companion/models.py`:
- Around line 242-248: There are two duplicate helpers (_clamp_int and
_clamp_int_or_default) implementing the same logic; remove the redundant one and
consolidate to a single implementation (prefer keeping _clamp_int_or_default),
then replace all usages of _clamp_int with _clamp_int_or_default (or vice versa)
so callers use the single function and the signatures match (minimum, maximum,
default, and value handling); ensure tests/type hints still compile after
removing the duplicate.
🪄 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: baa24e42-6118-4c25-9f25-c16098979645
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (51)
app/agent_server.pyapp/main_server.pymain_logic/agent_event_bus.pymain_logic/core.pyplugin/plugins/study_companion/__init__.pyplugin/plugins/study_companion/constants.pyplugin/plugins/study_companion/doc_exporter.pyplugin/plugins/study_companion/entry_export_support.pyplugin/plugins/study_companion/entry_knowledge_entries.pyplugin/plugins/study_companion/entry_notebook.pyplugin/plugins/study_companion/i18n/en.jsonplugin/plugins/study_companion/i18n/es.jsonplugin/plugins/study_companion/i18n/ja.jsonplugin/plugins/study_companion/i18n/ko.jsonplugin/plugins/study_companion/i18n/pt.jsonplugin/plugins/study_companion/i18n/ru.jsonplugin/plugins/study_companion/i18n/zh-CN.jsonplugin/plugins/study_companion/i18n/zh-TW.jsonplugin/plugins/study_companion/models.pyplugin/plugins/study_companion/plugin.tomlplugin/plugins/study_companion/screen_classifier.pyplugin/plugins/study_companion/service.pyplugin/plugins/study_companion/static/index.htmlplugin/plugins/study_companion/static/katex-render.jsplugin/plugins/study_companion/static/main.jsplugin/plugins/study_companion/static/math-parser.jsplugin/plugins/study_companion/static/style.cssplugin/plugins/study_companion/store_notebook.pyplugin/plugins/study_companion/store_schema.pyplugin/plugins/study_companion/study_ocr_pipeline.pyplugin/plugins/study_companion/surfaces/memory_shared.tsplugin/plugins/study_companion/surfaces/note_card.tsxplugin/plugins/study_companion/surfaces/note_editor.tsxplugin/plugins/study_companion/surfaces/note_search.tsxplugin/plugins/study_companion/surfaces/notebook_panel.tsxplugin/plugins/study_companion/surfaces/study_panel.tsxplugin/plugins/study_companion/tutor_llm_agent.pyplugin/plugins/study_companion/tutor_llm_agent_notebook.pyplugin/plugins/study_companion/voice_contracts.pyplugin/server/application/plugins/dispatch_service.pyplugin/server/application/plugins/voice_contracts.pyplugin/tests/unit/plugins/test_study_companion.pyplugin/tests/unit/plugins/test_study_companion_llm_prompts.pyplugin/tests/unit/plugins/test_study_companion_neko_commands.pyplugin/tests/unit/plugins/test_study_companion_phase9_ux.pyplugin/tests/unit/plugins/test_study_companion_study_ocr_pipeline.pyplugin/tests/unit/plugins/test_study_companion_voice_bridge.pyplugin/tests/unit/plugins/test_study_companion_voice_contracts.pyplugin/tests/unit/plugins/test_study_notebook.pypyproject.tomltests/unit/test_core_game_route_memory_contract.py
💤 Files with no reviewable changes (1)
- tests/unit/test_core_game_route_memory_contract.py
✅ Files skipped from review due to trivial changes (5)
- plugin/tests/unit/plugins/test_study_companion_voice_bridge.py
- plugin/plugins/study_companion/constants.py
- plugin/plugins/study_companion/i18n/pt.json
- plugin/plugins/study_companion/i18n/zh-TW.json
- plugin/server/application/plugins/dispatch_service.py
🚧 Files skipped from review as they are similar to previous changes (34)
- plugin/plugins/study_companion/surfaces/memory_shared.ts
- plugin/tests/unit/plugins/test_study_companion_llm_prompts.py
- plugin/plugins/study_companion/plugin.toml
- plugin/plugins/study_companion/i18n/zh-CN.json
- plugin/plugins/study_companion/static/main.js
- pyproject.toml
- plugin/tests/unit/plugins/test_study_companion_study_ocr_pipeline.py
- plugin/plugins/study_companion/surfaces/note_card.tsx
- plugin/plugins/study_companion/surfaces/note_search.tsx
- plugin/plugins/study_companion/voice_contracts.py
- app/main_server.py
- plugin/plugins/study_companion/tutor_llm_agent.py
- plugin/plugins/study_companion/entry_knowledge_entries.py
- plugin/plugins/study_companion/service.py
- plugin/plugins/study_companion/i18n/ru.json
- plugin/plugins/study_companion/i18n/ja.json
- plugin/plugins/study_companion/i18n/en.json
- plugin/plugins/study_companion/i18n/ko.json
- plugin/plugins/study_companion/static/katex-render.js
- plugin/tests/unit/plugins/test_study_companion.py
- plugin/server/application/plugins/voice_contracts.py
- plugin/plugins/study_companion/doc_exporter.py
- plugin/tests/unit/plugins/test_study_companion_voice_contracts.py
- plugin/plugins/study_companion/entry_export_support.py
- plugin/plugins/study_companion/store_schema.py
- plugin/plugins/study_companion/tutor_llm_agent_notebook.py
- plugin/plugins/study_companion/surfaces/notebook_panel.tsx
- plugin/plugins/study_companion/screen_classifier.py
- plugin/tests/unit/plugins/test_study_companion_phase9_ux.py
- plugin/tests/unit/plugins/test_study_notebook.py
- main_logic/agent_event_bus.py
- plugin/plugins/study_companion/surfaces/note_editor.tsx
- plugin/plugins/study_companion/study_ocr_pipeline.py
- plugin/plugins/study_companion/entry_notebook.py
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 2
♻️ Duplicate comments (3)
app/agent_server.py (2)
1654-1744:⚠️ Potential issue | 🟠 Major别在这里再维护第二份
voice_transcript契约喵。共享的 event type / event id / 仲裁逻辑已经在服务端
voice_contracts.py里定义好了,study_companion 侧也是按那套常量注册订阅的;这里继续手写VOICE_TRANSCRIPT_CUSTOM_EVENT_*,再自己重组一遍dispatch_results,会把跨边界契约拆成两份真相源,后面共享常量或结果 schema 一变,这里就会静默退化成no_subscribers或丢掉共享归一化结果喵。建议直接复用共享常量,并把原始dispatch_results直接交给共享仲裁函数处理喵。🐾 建议收敛到共享契约喵
+from plugin.server.application.plugins.voice_contracts import ( + VOICE_TRANSCRIPT_EVENT_TYPE as VOICE_TRANSCRIPT_CUSTOM_EVENT_TYPE, + VOICE_TRANSCRIPT_EVENT_ID as VOICE_TRANSCRIPT_CUSTOM_EVENT_ID, + arbitrate_voice_transcript_results, +) - -VOICE_TRANSCRIPT_CUSTOM_EVENT_TYPE = "voice_transcript" -VOICE_TRANSCRIPT_CUSTOM_EVENT_ID = "handle_transcript" VOICE_TRANSCRIPT_CUSTOM_EVENT_TIMEOUT_SECONDS = 1.0 ... def _voice_bridge_action_from_dispatch_results(dispatch_results: object) -> Dict[str, Any]: - if not isinstance(dispatch_results, list) or not dispatch_results: - return _voice_bridge_noop("no_subscribers") - - from plugin.server.application.plugins.voice_contracts import ( - arbitrate_voice_transcript_results, - ) - - arbitration_items: list[dict[str, object]] = [] - failure_count = 0 - for item in dispatch_results: - ... - if not arbitration_items: - return _voice_bridge_noop("no_handler_result", failures=failure_count) - payload = arbitrate_voice_transcript_results(arbitration_items) - if failure_count: - ... - return payload + return arbitrate_voice_transcript_results(dispatch_results)🤖 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 `@app/agent_server.py` around lines 1654 - 1744, This duplicates the voice_transcript contract (VOICE_TRANSCRIPT_CUSTOM_EVENT_TYPE/ID and local arbitration in _voice_bridge_action_from_dispatch_results) instead of reusing the shared contract; update _dispatch_voice_transcript_custom_event to import and use the shared constants and pass the raw dispatch_results into the shared arbiter (arbitrate_voice_transcript_results) rather than rebuilding items, remove or delegate _voice_transcript_custom_event_args to the shared arg/schema helper if one exists, and delete the local VOICE_TRANSCRIPT_CUSTOM_EVENT_* constants and the custom reconstruction logic in _voice_bridge_action_from_dispatch_results so the code delegates to the shared contract functions (refer to _dispatch_voice_transcript_custom_event, _voice_transcript_custom_event_args, _voice_bridge_action_from_dispatch_results, and arbitrate_voice_transcript_results to locate the changes).
1760-1767:⚠️ Potential issue | 🟠 Major实时转写路径不要在这里等插件冷启动喵。
这里把
voice_transcript_request卡在await _ensure_plugin_lifecycle_started()上了,而 1 秒超时预算只包住后面的 subscriber dispatch,不包生命周期启动本身喵。第一次 transcript 很容易等到插件起来了才拿到结果,但那时 cancel/prime 的时机已经过了,整条语音桥接就会变成过期决策喵。这个分支更稳的做法是直接回noop("plugin_warming_up"),把冷启动留给已有的 enable/restore 后台流程喵。😼 一个更稳的收口方式喵
else: - lifecycle_ready = bool(Modules.plugin_lifecycle_started) - if not lifecycle_ready: - lifecycle_ready = await _ensure_plugin_lifecycle_started() - - if not lifecycle_ready: - result = _voice_bridge_noop("plugin_lifecycle_start_failed") - else: - result = await _dispatch_voice_transcript_custom_event(event) + if not Modules.plugin_lifecycle_started: + result = _voice_bridge_noop("plugin_warming_up") + else: + result = await _dispatch_voice_transcript_custom_event(event)🤖 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 `@app/agent_server.py` around lines 1760 - 1767, The real-time transcript path must not block on plugin cold-start; replace the await of _ensure_plugin_lifecycle_started() in the branch that checks Modules.plugin_lifecycle_started so that if Modules.plugin_lifecycle_started is False you immediately return result = _voice_bridge_noop("plugin_warming_up") instead of awaiting _ensure_plugin_lifecycle_started(), otherwise proceed to await _dispatch_voice_transcript_custom_event(event); update the logic around lifecycle_ready, _ensure_plugin_lifecycle_started, _voice_bridge_noop and _dispatch_voice_transcript_custom_event in app/agent_server.py accordingly.main_logic/core.py (1)
2009-2022:⚠️ Potential issue | 🟠 Major | ⚡ Quick win
cancel_response失败时别把这条 transcript 也一起吞掉喵。这里即使
cancel_response不存在、不可调用,或者 await 时报错,末尾也还是return action喵。handle_input_transcript()在 Line 2261 收到"cancel_response"后会直接早退,所以这句用户转写后面的on_user_message、turn 计数、_publish_user_utterance_to_plugin_bus()和前端user_transcript都会被整条跳过喵。只有真的完成了 cancel side effect,才应该返回"cancel_response";失败时应该回落成普通 transcript 流程喵。🛠️ 一个局部修正方向喵
if action == "cancel_response": cancel_response = getattr(session_snapshot, "cancel_response", None) - if callable(cancel_response): - try: - if _session_changed(): - return "" - await cancel_response() - logger.debug("[%s] voice bridge cancelled current response", self.lanlan_name) - except asyncio.CancelledError: - raise - except Exception as exc: - logger.debug("[%s] voice bridge cancel skipped/failed: %s", self.lanlan_name, exc) - return action + if not callable(cancel_response): + return "" + try: + if _session_changed(): + return "" + await cancel_response() + if _session_changed(): + return "" + logger.debug("[%s] voice bridge cancelled current response", self.lanlan_name) + return action + except asyncio.CancelledError: + raise + except Exception as exc: + logger.debug("[%s] voice bridge cancel skipped/failed: %s", self.lanlan_name, exc) + return ""Also applies to: 2256-2262
🤖 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.py` around lines 2009 - 2022, The current branch returns action == "cancel_response" regardless of whether session_snapshot.cancel_response existed, was callable, or succeeded, causing handle_input_transcript() to short-circuit and skip normal transcript handling; change the logic in the cancel_response handling (around the action variable and the block using session_snapshot.cancel_response, _session_changed(), and await cancel_response()) so that you only return "cancel_response" after the cancel_response callable was found and awaited successfully (no exception and _session_changed() false); on any failure (missing/non-callable cancel_response, _session_changed() true, or exception during await) fall back to the normal transcript flow by NOT returning action and allowing subsequent processing (on_user_message, turn counting, _publish_user_utterance_to_plugin_bus, and user_transcript) to proceed. Ensure you reference session_snapshot, cancel_response, _session_changed(), and the surrounding return action to implement this conditional return behavior.
🧹 Nitpick comments (1)
plugin/plugins/study_companion/models.py (1)
242-248: 💤 Low value
_clamp_int和_clamp_int_or_default功能重复了喵~第 143-153 行已经有一个
_clamp_int_or_default做同样的事情,这里又定义了一个几乎一样的_clamp_int,有点多余呢。可以考虑统一用一个就好了喵~🤖 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 `@plugin/plugins/study_companion/models.py` around lines 242 - 248, There are two duplicate helpers (_clamp_int and _clamp_int_or_default) implementing the same logic; remove the redundant one and consolidate to a single implementation (prefer keeping _clamp_int_or_default), then replace all usages of _clamp_int with _clamp_int_or_default (or vice versa) so callers use the single function and the signatures match (minimum, maximum, default, and value handling); ensure tests/type hints still compile after removing the duplicate.
🤖 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.py`:
- Around line 1977-2007: The code only checks session identity (self.session is
session_snapshot) to ignore stale bridge results but multiple concurrent STT
bridge requests can run for the same session; record and compare the per-request
realtime_voice_request_id to avoid applying late results to a newer turn: add an
instance field (e.g. self._latest_voice_transcript_request_id) initialized in
__init__, set it immediately when handle_input_transcript generates
realtime_voice_request_id, and then before and after awaiting
publish_voice_transcript_request_reliably (and before calling cancel_response()
/ prime_context()) verify the awaited request_id matches
self._latest_voice_transcript_request_id and bail out if not; update or add
similar checks around the blocks referenced by
publish_voice_transcript_request_reliably, _session_changed(),
cancel_response(), and prime_context().
In `@plugin/plugins/study_companion/static/index.html`:
- Line 12: The ARIA localization attribute used (data-i18n-aria-label) is not
recognized by our i18n injection logic, so replace every occurrence of
data-i18n-aria-label="..." with the project-standard data-i18n-aria="..."
(preserving the same translation key) for the elements mentioned (e.g., the
<section class="study-toolbar"> and the other elements at the noted locations)
so that i18n-i18next.js (which uses
document.querySelectorAll('[data-i18n-aria]')) will correctly write the
localized aria-label; keep the existing aria-label values as safe fallbacks if
present.
---
Duplicate comments:
In `@app/agent_server.py`:
- Around line 1654-1744: This duplicates the voice_transcript contract
(VOICE_TRANSCRIPT_CUSTOM_EVENT_TYPE/ID and local arbitration in
_voice_bridge_action_from_dispatch_results) instead of reusing the shared
contract; update _dispatch_voice_transcript_custom_event to import and use the
shared constants and pass the raw dispatch_results into the shared arbiter
(arbitrate_voice_transcript_results) rather than rebuilding items, remove or
delegate _voice_transcript_custom_event_args to the shared arg/schema helper if
one exists, and delete the local VOICE_TRANSCRIPT_CUSTOM_EVENT_* constants and
the custom reconstruction logic in _voice_bridge_action_from_dispatch_results so
the code delegates to the shared contract functions (refer to
_dispatch_voice_transcript_custom_event, _voice_transcript_custom_event_args,
_voice_bridge_action_from_dispatch_results, and
arbitrate_voice_transcript_results to locate the changes).
- Around line 1760-1767: The real-time transcript path must not block on plugin
cold-start; replace the await of _ensure_plugin_lifecycle_started() in the
branch that checks Modules.plugin_lifecycle_started so that if
Modules.plugin_lifecycle_started is False you immediately return result =
_voice_bridge_noop("plugin_warming_up") instead of awaiting
_ensure_plugin_lifecycle_started(), otherwise proceed to await
_dispatch_voice_transcript_custom_event(event); update the logic around
lifecycle_ready, _ensure_plugin_lifecycle_started, _voice_bridge_noop and
_dispatch_voice_transcript_custom_event in app/agent_server.py accordingly.
In `@main_logic/core.py`:
- Around line 2009-2022: The current branch returns action == "cancel_response"
regardless of whether session_snapshot.cancel_response existed, was callable, or
succeeded, causing handle_input_transcript() to short-circuit and skip normal
transcript handling; change the logic in the cancel_response handling (around
the action variable and the block using session_snapshot.cancel_response,
_session_changed(), and await cancel_response()) so that you only return
"cancel_response" after the cancel_response callable was found and awaited
successfully (no exception and _session_changed() false); on any failure
(missing/non-callable cancel_response, _session_changed() true, or exception
during await) fall back to the normal transcript flow by NOT returning action
and allowing subsequent processing (on_user_message, turn counting,
_publish_user_utterance_to_plugin_bus, and user_transcript) to proceed. Ensure
you reference session_snapshot, cancel_response, _session_changed(), and the
surrounding return action to implement this conditional return behavior.
---
Nitpick comments:
In `@plugin/plugins/study_companion/models.py`:
- Around line 242-248: There are two duplicate helpers (_clamp_int and
_clamp_int_or_default) implementing the same logic; remove the redundant one and
consolidate to a single implementation (prefer keeping _clamp_int_or_default),
then replace all usages of _clamp_int with _clamp_int_or_default (or vice versa)
so callers use the single function and the signatures match (minimum, maximum,
default, and value handling); ensure tests/type hints still compile after
removing the duplicate.
🪄 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: baa24e42-6118-4c25-9f25-c16098979645
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (51)
app/agent_server.pyapp/main_server.pymain_logic/agent_event_bus.pymain_logic/core.pyplugin/plugins/study_companion/__init__.pyplugin/plugins/study_companion/constants.pyplugin/plugins/study_companion/doc_exporter.pyplugin/plugins/study_companion/entry_export_support.pyplugin/plugins/study_companion/entry_knowledge_entries.pyplugin/plugins/study_companion/entry_notebook.pyplugin/plugins/study_companion/i18n/en.jsonplugin/plugins/study_companion/i18n/es.jsonplugin/plugins/study_companion/i18n/ja.jsonplugin/plugins/study_companion/i18n/ko.jsonplugin/plugins/study_companion/i18n/pt.jsonplugin/plugins/study_companion/i18n/ru.jsonplugin/plugins/study_companion/i18n/zh-CN.jsonplugin/plugins/study_companion/i18n/zh-TW.jsonplugin/plugins/study_companion/models.pyplugin/plugins/study_companion/plugin.tomlplugin/plugins/study_companion/screen_classifier.pyplugin/plugins/study_companion/service.pyplugin/plugins/study_companion/static/index.htmlplugin/plugins/study_companion/static/katex-render.jsplugin/plugins/study_companion/static/main.jsplugin/plugins/study_companion/static/math-parser.jsplugin/plugins/study_companion/static/style.cssplugin/plugins/study_companion/store_notebook.pyplugin/plugins/study_companion/store_schema.pyplugin/plugins/study_companion/study_ocr_pipeline.pyplugin/plugins/study_companion/surfaces/memory_shared.tsplugin/plugins/study_companion/surfaces/note_card.tsxplugin/plugins/study_companion/surfaces/note_editor.tsxplugin/plugins/study_companion/surfaces/note_search.tsxplugin/plugins/study_companion/surfaces/notebook_panel.tsxplugin/plugins/study_companion/surfaces/study_panel.tsxplugin/plugins/study_companion/tutor_llm_agent.pyplugin/plugins/study_companion/tutor_llm_agent_notebook.pyplugin/plugins/study_companion/voice_contracts.pyplugin/server/application/plugins/dispatch_service.pyplugin/server/application/plugins/voice_contracts.pyplugin/tests/unit/plugins/test_study_companion.pyplugin/tests/unit/plugins/test_study_companion_llm_prompts.pyplugin/tests/unit/plugins/test_study_companion_neko_commands.pyplugin/tests/unit/plugins/test_study_companion_phase9_ux.pyplugin/tests/unit/plugins/test_study_companion_study_ocr_pipeline.pyplugin/tests/unit/plugins/test_study_companion_voice_bridge.pyplugin/tests/unit/plugins/test_study_companion_voice_contracts.pyplugin/tests/unit/plugins/test_study_notebook.pypyproject.tomltests/unit/test_core_game_route_memory_contract.py
💤 Files with no reviewable changes (1)
- tests/unit/test_core_game_route_memory_contract.py
✅ Files skipped from review due to trivial changes (5)
- plugin/tests/unit/plugins/test_study_companion_voice_bridge.py
- plugin/plugins/study_companion/constants.py
- plugin/plugins/study_companion/i18n/pt.json
- plugin/plugins/study_companion/i18n/zh-TW.json
- plugin/server/application/plugins/dispatch_service.py
🚧 Files skipped from review as they are similar to previous changes (34)
- plugin/plugins/study_companion/surfaces/memory_shared.ts
- plugin/tests/unit/plugins/test_study_companion_llm_prompts.py
- plugin/plugins/study_companion/plugin.toml
- plugin/plugins/study_companion/i18n/zh-CN.json
- plugin/plugins/study_companion/static/main.js
- pyproject.toml
- plugin/tests/unit/plugins/test_study_companion_study_ocr_pipeline.py
- plugin/plugins/study_companion/surfaces/note_card.tsx
- plugin/plugins/study_companion/surfaces/note_search.tsx
- plugin/plugins/study_companion/voice_contracts.py
- app/main_server.py
- plugin/plugins/study_companion/tutor_llm_agent.py
- plugin/plugins/study_companion/entry_knowledge_entries.py
- plugin/plugins/study_companion/service.py
- plugin/plugins/study_companion/i18n/ru.json
- plugin/plugins/study_companion/i18n/ja.json
- plugin/plugins/study_companion/i18n/en.json
- plugin/plugins/study_companion/i18n/ko.json
- plugin/plugins/study_companion/static/katex-render.js
- plugin/tests/unit/plugins/test_study_companion.py
- plugin/server/application/plugins/voice_contracts.py
- plugin/plugins/study_companion/doc_exporter.py
- plugin/tests/unit/plugins/test_study_companion_voice_contracts.py
- plugin/plugins/study_companion/entry_export_support.py
- plugin/plugins/study_companion/store_schema.py
- plugin/plugins/study_companion/tutor_llm_agent_notebook.py
- plugin/plugins/study_companion/surfaces/notebook_panel.tsx
- plugin/plugins/study_companion/screen_classifier.py
- plugin/tests/unit/plugins/test_study_companion_phase9_ux.py
- plugin/tests/unit/plugins/test_study_notebook.py
- main_logic/agent_event_bus.py
- plugin/plugins/study_companion/surfaces/note_editor.tsx
- plugin/plugins/study_companion/study_ocr_pipeline.py
- plugin/plugins/study_companion/entry_notebook.py
🛑 Comments failed to post (2)
main_logic/core.py (1)
1977-2007:
⚠️ Potential issue | 🟠 Major | ⚡ Quick win别把过期的桥接结果打到同一个 realtime session 的更新 turn 上喵。
这里现在只用
self.session is session_snapshot判断结果是否过期,但同一个 session 内本来就可能并发跑多条 STT 桥接请求喵。A 条转写桥接慢一点时,B 条已经先完成并推进了更新的轮次,A 的迟到结果还是会继续cancel_response()/prime_context(),于是把旧决定应用到新 turn 上喵。既然这里已经生成了realtime_voice_request_id,建议把它作为“当前最新桥接请求”记下来,并在 await 返回前后都核对 request_id 仍然匹配后再执行 side effect 喵。🧩 一个最小收口方向喵
async def _dispatch_voice_transcript_bridge( self, transcript: str, *, request_id: str = "", ) -> str: """Let plugin-side voice filters decide whether to cancel or prime context.""" session_snapshot = self.session + request_id_snapshot = request_id + def _request_stale() -> bool: + return bool( + request_id_snapshot + and getattr(self, "_latest_voice_transcript_request_id", "") != request_id_snapshot + ) + metadata: dict[str, Any] = { "session_type": type(session_snapshot).__name__ if session_snapshot else "", "voice_source": True, "source": "realtime_stt", } @@ - if _session_changed(): + if _session_changed() or _request_stale(): return "" @@ - if _session_changed(): + if _session_changed() or _request_stale(): return "" await cancel_response() + if _session_changed() or _request_stale(): + return "" @@ - if _session_changed(): + if _session_changed() or _request_stale(): return "" await prime_context(context_text, skipped=skipped) + if _session_changed() or _request_stale(): + return ""# 在 __init__ 里补一个字段 self._latest_voice_transcript_request_id = "" # 在 handle_input_transcript 里生成 request_id 后立刻刷新 if realtime_voice_request_id: self._latest_voice_transcript_request_id = realtime_voice_request_idAlso applies to: 2009-2051, 2200-2260
🤖 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.py` around lines 1977 - 2007, The code only checks session identity (self.session is session_snapshot) to ignore stale bridge results but multiple concurrent STT bridge requests can run for the same session; record and compare the per-request realtime_voice_request_id to avoid applying late results to a newer turn: add an instance field (e.g. self._latest_voice_transcript_request_id) initialized in __init__, set it immediately when handle_input_transcript generates realtime_voice_request_id, and then before and after awaiting publish_voice_transcript_request_reliably (and before calling cancel_response() / prime_context()) verify the awaited request_id matches self._latest_voice_transcript_request_id and bail out if not; update or add similar checks around the blocks referenced by publish_voice_transcript_request_reliably, _session_changed(), cancel_response(), and prime_context().plugin/plugins/study_companion/static/index.html (1)
12-12:
⚠️ Potential issue | 🟠 Major | ⚡ Quick win
data-i18n-aria-label属性名不符合项目约定,aria 文案不会被本地化喵。Line 12、Line 29、Line 44、Line 68、Line 77、Line 88 使用了
data-i18n-aria-label,现有 i18n 注入逻辑不会识别它,最终会一直落到硬编码英文aria-label,无障碍多语言会失效喵。🔧 建议修改
- <section class="study-toolbar" data-i18n-aria-label="ui.label.study_controls" aria-label="Study companion controls"> + <section class="study-toolbar" data-i18n-aria="ui.label.study_controls" aria-label="Study companion controls"> ... - <section class="study-meta" data-i18n-aria-label="ui.label.study_state" aria-label="Study state"> + <section class="study-meta" data-i18n-aria="ui.label.study_state" aria-label="Study state"> ... - <section class="memory-panel" aria-live="polite" data-i18n-aria-label="ui.memory.title" aria-label="Memory Deck"> + <section class="memory-panel" aria-live="polite" data-i18n-aria="ui.memory.title" aria-label="Memory Deck"> ... - <section class="study-workspace" data-i18n-aria-label="ui.label.study_workspace" aria-label="Study workspace"> + <section class="study-workspace" data-i18n-aria="ui.label.study_workspace" aria-label="Study workspace"> ... - <section class="question-panel" aria-live="polite" data-i18n-aria-label="ui.label.question_panel" aria-label="Question panel"> + <section class="question-panel" aria-live="polite" data-i18n-aria="ui.label.question_panel" aria-label="Question panel"> ... - <section class="reply-panel" aria-live="polite" data-i18n-aria-label="ui.label.reply_panel" aria-label="Reply panel"> + <section class="reply-panel" aria-live="polite" data-i18n-aria="ui.label.reply_panel" aria-label="Reply panel">Based on learnings: 项目约定使用
data-i18n-aria="<key>",并由i18n-i18next.js的document.querySelectorAll('[data-i18n-aria]')写入aria-label。Also applies to: 29-29, 44-44, 68-68, 77-77, 88-88
🤖 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 `@plugin/plugins/study_companion/static/index.html` at line 12, The ARIA localization attribute used (data-i18n-aria-label) is not recognized by our i18n injection logic, so replace every occurrence of data-i18n-aria-label="..." with the project-standard data-i18n-aria="..." (preserving the same translation key) for the elements mentioned (e.g., the <section class="study-toolbar"> and the other elements at the noted locations) so that i18n-i18next.js (which uses document.querySelectorAll('[data-i18n-aria]')) will correctly write the localized aria-label; keep the existing aria-label values as safe fallbacks if present.
Implement the complete study_companion plugin capabilities: Notebook System: - Note data model, SQLite schema, FTS5 search, CRUD store (store_notebook.py, store_schema.py) - Plugin entries: note create/update/query/cross-module search (entry_notebook.py) - Frontend UI: note_card, note_editor, note_search, notebook_panel - AI note expansion and summarization (tutor_llm_agent_notebook.py) - KaTeX math rendering with full font set and CSS/JS - jieba Chinese text segmentation for search (pyproject.toml dep) Voice Interaction: - Voice transcript filter with subject detection and keyword matching (voice_filter.py) - Voice contracts for cancel_response / prime_context actions (voice_contracts.py) - Integration with neko voice bridge infrastructure Localization: - 10+ language i18n support for notebook, voice, UI entries This is Part 2 of the original PR Project-N-E-K-O#1582 split. Depends on Part 1 (voice bridge + plugin dispatch infrastructure).
fix: use shared voice_contracts and dispatch_service from infra layer - Replace plugin/plugins/study_companion/voice_contracts.py with a thin re-export from plugin/server/application/plugins/voice_contracts.py. Plugin-internal imports (from .voice_contracts) continue to work unchanged. - Pull updated dispatch_service.py with _validate_timeout helper, CancelledError re-raise, and TimeoutError-specific handling from the infrastructure layer (sibling neko-core PR). - Add the shared voice_contracts.py at plugin/server/ level (the canonical location for voice contract definitions that both host and plugin code can depend on). @
f3c3f0c to
4d7ae33
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4d7ae335f6
ℹ️ 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".
| _BROWSER_TITLE_KEYWORDS = ( | ||
| " chrome", | ||
| "google chrome", | ||
| "mozilla firefox", | ||
| " firefox", | ||
| "microsoft edge", | ||
| "safari", | ||
| "arc browser", | ||
| "chromium", | ||
| "brave", | ||
| "opera", |
There was a problem hiding this comment.
Restore page-title browser keywords
When awareness runs on platforms where the active-window lookup returns only the document/page title (for example the macOS name of first window path), common study browser pages such as GitHub, Wikipedia, or Stack Overflow no longer contain Chrome/Firefox/Safari and now classify as other; the previous rule set handled those page-title keywords as web_page. In title_first awareness this makes those pages count as idle/other and can skew distraction/focus tracking, so keep the page-title keywords or include the app name before classifying.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 592eeac8d5
ℹ️ 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".
| " chrome", | ||
| "google chrome", | ||
| "mozilla firefox", | ||
| " firefox", | ||
| "microsoft edge", |
There was a problem hiding this comment.
Restore bare browser app-name matches
When the active-window provider returns only the application name (for example Chrome, Firefox, or Edge on some OS/window-manager paths), these new browser keywords no longer match because Chrome/Firefox require a leading space and Edge only matches microsoft edge. The previous classifier matched the bare browser names, so awareness now records those browser sessions as other, skewing focus/distraction tracking whenever no page title is available.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
🧹 Nitpick comments (2)
plugin/plugins/study_companion/static/katex-render.js (1)
52-58: 💤 Low value导出的
splitByMath在 math-parser.js 加载失败时可能为undefined喵~哼,虽然本喵知道
renderMathInText已经做了防御性检查(Line 41-43),但是直接把mathParser.splitByMath导出到window.__studyCompanionMath上,如果 math-parser.js 没加载成功的话,外部调用者可就要吃TypeError啦!笨蛋主人应该考虑给个兜底函数喵~♻️ 可选的防御性改进喵
window.__studyCompanionMath = { escapeHTML, normalizeLatexForKatex, - splitByMath: mathParser.splitByMath, + splitByMath: typeof mathParser.splitByMath === 'function' + ? mathParser.splitByMath + : (text) => [{ type: 'text', value: String(text || '') }], renderMathInText, };🤖 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 `@plugin/plugins/study_companion/static/katex-render.js` around lines 52 - 58, The export assigns mathParser.splitByMath directly to window.__studyCompanionMath which will be undefined if math-parser.js fails to load; update the assignment to provide a safe fallback: check mathParser && typeof mathParser.splitByMath === 'function' and otherwise assign a defensive stub (e.g., a function that returns the input unchanged or throws a clear error) for splitByMath; keep renderMathInText and other helpers as-is but ensure window.__studyCompanionMath.splitByMath always exists to avoid TypeError in external callers.plugin/plugins/study_companion/store_notebook.py (1)
115-117: 💤 Low valueLIKE 通配符未转义,可能导致搜索行为不符合预期喵~
如果用户搜索的文本包含
%或_,这些字符会被当作 LIKE 通配符处理而不是字面值匹配。虽然不是注入问题(已参数化),但会导致匹配结果超出用户预期喵。🐱 可选的转义修复喵
def _like_pattern(query: str) -> str: text = str(query or "").strip() + # Escape LIKE wildcards + text = text.replace("\\", "\\\\").replace("%", "\\%").replace("_", "\\_") return f"%{text}%"注意:使用转义后需要在 SQL 查询中添加
ESCAPE '\\'子句喵~🤖 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 `@plugin/plugins/study_companion/store_notebook.py` around lines 115 - 117, The _like_pattern function currently wraps user input with % without escaping LIKE wildcards, so % and _ in query will act as wildcards; update _like_pattern to escape % and _ (and the escape char itself, e.g., backslash) in the input before surrounding with % so the pattern matches literals, and ensure any SQL that uses this pattern (queries calling _like_pattern) append the corresponding ESCAPE '\\' clause in their WHERE ... LIKE ... ESCAPE '\\' to make the escape semantics effective.
🤖 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 `@plugin/plugins/study_companion/static/katex-render.js`:
- Around line 52-58: The export assigns mathParser.splitByMath directly to
window.__studyCompanionMath which will be undefined if math-parser.js fails to
load; update the assignment to provide a safe fallback: check mathParser &&
typeof mathParser.splitByMath === 'function' and otherwise assign a defensive
stub (e.g., a function that returns the input unchanged or throws a clear error)
for splitByMath; keep renderMathInText and other helpers as-is but ensure
window.__studyCompanionMath.splitByMath always exists to avoid TypeError in
external callers.
In `@plugin/plugins/study_companion/store_notebook.py`:
- Around line 115-117: The _like_pattern function currently wraps user input
with % without escaping LIKE wildcards, so % and _ in query will act as
wildcards; update _like_pattern to escape % and _ (and the escape char itself,
e.g., backslash) in the input before surrounding with % so the pattern matches
literals, and ensure any SQL that uses this pattern (queries calling
_like_pattern) append the corresponding ESCAPE '\\' clause in their WHERE ...
LIKE ... ESCAPE '\\' to make the escape semantics effective.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 365a5b13-f0ac-417a-be1c-ab46c49134d4
⛔ Files ignored due to path filters (1)
uv.lockis excluded by!**/*.lock
📒 Files selected for processing (53)
app/agent_server.pyapp/main_server.pymain_logic/agent_event_bus.pymain_logic/core.pyplugin/plugins/study_companion/__init__.pyplugin/plugins/study_companion/constants.pyplugin/plugins/study_companion/doc_exporter.pyplugin/plugins/study_companion/entry_export_support.pyplugin/plugins/study_companion/entry_knowledge_entries.pyplugin/plugins/study_companion/entry_notebook.pyplugin/plugins/study_companion/i18n/en.jsonplugin/plugins/study_companion/i18n/es.jsonplugin/plugins/study_companion/i18n/ja.jsonplugin/plugins/study_companion/i18n/ko.jsonplugin/plugins/study_companion/i18n/pt.jsonplugin/plugins/study_companion/i18n/ru.jsonplugin/plugins/study_companion/i18n/zh-CN.jsonplugin/plugins/study_companion/i18n/zh-TW.jsonplugin/plugins/study_companion/models.pyplugin/plugins/study_companion/plugin.tomlplugin/plugins/study_companion/screen_classifier.pyplugin/plugins/study_companion/service.pyplugin/plugins/study_companion/static/i18n.jsplugin/plugins/study_companion/static/index.htmlplugin/plugins/study_companion/static/katex-render.jsplugin/plugins/study_companion/static/main.jsplugin/plugins/study_companion/static/math-parser.jsplugin/plugins/study_companion/static/style.cssplugin/plugins/study_companion/store_notebook.pyplugin/plugins/study_companion/store_schema.pyplugin/plugins/study_companion/study_ocr_pipeline.pyplugin/plugins/study_companion/surfaces/memory_shared.tsplugin/plugins/study_companion/surfaces/note_card.tsxplugin/plugins/study_companion/surfaces/note_editor.tsxplugin/plugins/study_companion/surfaces/note_search.tsxplugin/plugins/study_companion/surfaces/notebook_panel.tsxplugin/plugins/study_companion/surfaces/study_panel.tsxplugin/plugins/study_companion/tutor_llm_agent.pyplugin/plugins/study_companion/tutor_llm_agent_notebook.pyplugin/plugins/study_companion/voice_contracts.pyplugin/server/application/plugins/dispatch_service.pyplugin/server/application/plugins/voice_contracts.pyplugin/tests/unit/plugins/test_study_companion.pyplugin/tests/unit/plugins/test_study_companion_llm_prompts.pyplugin/tests/unit/plugins/test_study_companion_neko_commands.pyplugin/tests/unit/plugins/test_study_companion_phase9_ux.pyplugin/tests/unit/plugins/test_study_companion_screen_classifier.pyplugin/tests/unit/plugins/test_study_companion_study_ocr_pipeline.pyplugin/tests/unit/plugins/test_study_companion_voice_bridge.pyplugin/tests/unit/plugins/test_study_companion_voice_contracts.pyplugin/tests/unit/plugins/test_study_notebook.pypyproject.tomltests/unit/test_core_game_route_memory_contract.py
✅ Files skipped from review due to trivial changes (8)
- pyproject.toml
- plugin/plugins/study_companion/i18n/ru.json
- plugin/plugins/study_companion/i18n/pt.json
- plugin/plugins/study_companion/i18n/ko.json
- plugin/plugins/study_companion/i18n/zh-TW.json
- plugin/plugins/study_companion/voice_contracts.py
- plugin/plugins/study_companion/i18n/en.json
- plugin/plugins/study_companion/i18n/es.json
🚧 Files skipped from review as they are similar to previous changes (39)
- plugin/plugins/study_companion/surfaces/memory_shared.ts
- plugin/tests/unit/plugins/test_study_companion_llm_prompts.py
- plugin/plugins/study_companion/entry_knowledge_entries.py
- plugin/plugins/study_companion/constants.py
- plugin/plugins/study_companion/entry_export_support.py
- plugin/tests/unit/plugins/test_study_companion_voice_bridge.py
- plugin/plugins/study_companion/static/main.js
- plugin/tests/unit/plugins/test_study_companion.py
- plugin/tests/unit/plugins/test_study_companion_neko_commands.py
- plugin/plugins/study_companion/i18n/zh-CN.json
- plugin/plugins/study_companion/service.py
- plugin/tests/unit/plugins/test_study_companion_study_ocr_pipeline.py
- plugin/plugins/study_companion/plugin.toml
- plugin/plugins/study_companion/static/style.css
- plugin/plugins/study_companion/surfaces/note_card.tsx
- plugin/tests/unit/plugins/test_study_companion_voice_contracts.py
- app/main_server.py
- plugin/plugins/study_companion/i18n/ja.json
- plugin/plugins/study_companion/surfaces/notebook_panel.tsx
- plugin/plugins/study_companion/static/math-parser.js
- plugin/plugins/study_companion/surfaces/note_search.tsx
- main_logic/agent_event_bus.py
- plugin/plugins/study_companion/screen_classifier.py
- plugin/plugins/study_companion/doc_exporter.py
- plugin/server/application/plugins/dispatch_service.py
- plugin/plugins/study_companion/tutor_llm_agent.py
- plugin/plugins/study_companion/store_schema.py
- plugin/plugins/study_companion/surfaces/note_editor.tsx
- plugin/tests/unit/plugins/test_study_companion_phase9_ux.py
- app/agent_server.py
- plugin/plugins/study_companion/tutor_llm_agent_notebook.py
- plugin/tests/unit/plugins/test_study_notebook.py
- plugin/plugins/study_companion/models.py
- tests/unit/test_core_game_route_memory_contract.py
- plugin/plugins/study_companion/surfaces/study_panel.tsx
- main_logic/core.py
- plugin/plugins/study_companion/entry_notebook.py
- plugin/plugins/study_companion/study_ocr_pipeline.py
- plugin/plugins/study_companion/init.py
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fcebbd5f9d
ℹ️ 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".
| if notes: | ||
| return notes |
There was a problem hiding this comment.
Fall back to LIKE when FTS returns partial CJK results
When a search query has both FTS matches and substring-only matches, this early return drops the substring matches. For example, SQLite FTS5 with the default tokenizer matches a note titled 导数 for query 导数, but not another note whose content is 导数描述瞬时变化率; because notes is non-empty, the LIKE fallback that would find the second note is skipped. This makes notebook search incomplete for common CJK substring searches whenever at least one exact-token result exists.
Useful? React with 👍 / 👎.
| if any(keyword in title for keyword in _BROWSER_PAGE_TITLE_KEYWORDS): | ||
| return "web_page" |
There was a problem hiding this comment.
Check code-editor titles before browser page keywords
When the active-window title contains both a browser-page keyword and an editor app name, such as GitHub Pull Requests - Visual Studio Code, this branch classifies it as web_page before the later visual studio code check can classify it as code_editor. The previous scoring logic let the more specific editor keyword win, so focus tracking can now treat editor work as browsing for those VS Code/Cursor windows.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 50d6676f5e
ℹ️ 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".
| ) | ||
| conn.execute( | ||
| """ | ||
| CREATE TABLE IF NOT EXISTS notes ( |
There was a problem hiding this comment.
Add notebook tables to reset and export paths
Creating these new note tables adds user data that the existing maintenance paths do not know about: StudyStore.export_json() still returns a hard-coded set of tables without notebooks/notes, and _PURGE_TABLES likewise omits them. As a result, users who back up/export study data silently lose their notebook notes, and users who run the store reset/purge flow keep the notes behind even though other study records are deleted.
Useful? React with 👍 / 👎.
这个 PR 做了什么这个 PR 在 study_companion 插件内新增了完整的伴学笔记本和伴学语音交互能力,111 个文件,全部收拢在 笔记本系统
语音交互
为什么要这样做核心目标是把"学习内容沉淀"和"伴学语音判断"全部收敛到插件内部,不让宿主承载任何伴学业务语义。 笔记本学习过程中猫娘会讲解、问答、复习——这些内容如果不沉淀,用户无法复习回顾。笔记本系统让 bot 可以把讲解内容自动整理成结构化笔记(AI generate),用户也可以手动创作。KaTeX 支持让数学/化学/物理公式正常渲染。 语音过滤实时语音识别是连续的,用户可能在看视频、自言自语、念题目——如果每次识别到文字都触发猫娘打断当前回复,体验会很差。 边界设计host(Part 1 #1612)只做通道:转发 transcript → 等待结果 → 执行 session action。插件(本 PR)负责所有决策。这样:
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
plugin/tests/unit/plugins/test_study_companion.py (1)
435-536: ⚡ Quick win建议把
store.close()放进finally,避免断言失败时连接泄漏喵。现在 Line 536 只在成功路径执行;一旦中途断言失败,数据库连接不会及时释放,可能影响后续用例稳定性喵。
🤖 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 `@plugin/tests/unit/plugins/test_study_companion.py` around lines 435 - 536, The test test_study_store_round_trip_and_export currently calls store.close() only at the end so a failing assertion can leak the DB connection; wrap the main test body (everything after store.open()) in a try/finally and call store.close() in the finally block (or use a context manager if StudyStore supports __enter__/__exit__) so store.close() always runs even on failure.
🤖 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 `@plugin/tests/unit/plugins/test_study_companion.py`:
- Around line 435-536: The test test_study_store_round_trip_and_export currently
calls store.close() only at the end so a failing assertion can leak the DB
connection; wrap the main test body (everything after store.open()) in a
try/finally and call store.close() in the finally block (or use a context
manager if StudyStore supports __enter__/__exit__) so store.close() always runs
even on failure.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: b14f7736-9743-4541-9138-8b864ba123cc
📒 Files selected for processing (6)
plugin/plugins/study_companion/store.pyplugin/plugins/study_companion/store_maintenance.pyplugin/plugins/study_companion/store_notebook.pyplugin/tests/unit/plugins/test_study_companion.pyplugin/tests/unit/plugins/test_study_habit_store.pyplugin/tests/unit/plugins/test_study_notebook.py
🚧 Files skipped from review as they are similar to previous changes (2)
- plugin/tests/unit/plugins/test_study_notebook.py
- plugin/plugins/study_companion/store_notebook.py
概述 (Part 2/2 — 伴学插件本体)
变更范围(111 文件,仅限于
plugin/plugins/study_companion/+ 依赖)笔记本系统
语音交互
审查修复记录
plugin/server/.../voice_contracts.py重导出(边界解耦)_validate_timeout、CancelledError、TimeoutError 修复Summary by CodeRabbit
New Features
Improvements
Tests