Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
审阅者指南此 PR 引入了一个由环境变量 Web UI 中演示模式行为开关的流程图flowchart TD
DEMO_env[DEMO=1 environment] --> is_demo_mode
is_demo_mode -->|true| ws_live_screenshot
is_demo_mode -->|true| ws_live_control
is_demo_mode -->|true| simulator_start
is_demo_mode -->|true| ocr_server_start
is_demo_mode -->|true| demo_device_id
is_demo_mode -->|true| demo_run_process
ws_live_screenshot[[ws_live_screenshot]] --> disabled_preview[Send error and close websocket]
ws_live_control[[ws_live_control]] --> disabled_control[Send error and close websocket]
simulator_start[[WebUI._simulator_start]] --> skip_simulator[Skip simulator.start]
ocr_server_start[[startup start_ocr_server_process]] --> skip_ocr[Do not start OCR server]
demo_device_id[[get_device_id in UI]] --> use_demo_text[Use DEMO_DEVICE_ID_TEXT]
demo_run_process[[run_process]] --> demo_logs[Log countdown and exit]
is_demo_mode -->|false| normal_flow[Normal behavior for all components]
文件级变更
技巧和命令与 Sourcery 交互
自定义你的使用体验访问你的 控制面板 以:
获取帮助Original review guide in EnglishReviewer's GuideThis PR introduces a demo mode controlled by the DEMO=1 environment variable that disables live preview/control, avoids starting heavy services (simulator, OCR server, device restriction checks), and replaces real device IDs and process behavior with demo-safe messages and flow. Flow diagram for demo mode behavior gating in web UIflowchart TD
DEMO_env[DEMO=1 environment] --> is_demo_mode
is_demo_mode -->|true| ws_live_screenshot
is_demo_mode -->|true| ws_live_control
is_demo_mode -->|true| simulator_start
is_demo_mode -->|true| ocr_server_start
is_demo_mode -->|true| demo_device_id
is_demo_mode -->|true| demo_run_process
ws_live_screenshot[[ws_live_screenshot]] --> disabled_preview[Send error and close websocket]
ws_live_control[[ws_live_control]] --> disabled_control[Send error and close websocket]
simulator_start[[WebUI._simulator_start]] --> skip_simulator[Skip simulator.start]
ocr_server_start[[startup start_ocr_server_process]] --> skip_ocr[Do not start OCR server]
demo_device_id[[get_device_id in UI]] --> use_demo_text[Use DEMO_DEVICE_ID_TEXT]
demo_run_process[[run_process]] --> demo_logs[Log countdown and exit]
is_demo_mode -->|false| normal_flow[Normal behavior for all components]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Code Review
This pull request introduces a demo mode (DEMO=1) to restrict features like live screenshots, live controls, simulator startup, and OCR server processes, while displaying a placeholder device ID. The review feedback highlights a potential race condition in ProcessManager where the log queue handler thread might terminate before processing the final demo log message, and suggests deduplicating the is_demo_mode() helper function by centralizing it in a shared utility module.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| if update_tail_hit: | ||
| return 4 | ||
| return 2 | ||
| elif "此版本为演示用途" in s: |
There was a problem hiding this comment.
这里存在一个潜在的竞争条件(Race Condition)。在 ProcessManager 中,_thread_log_queue_handler 线程是通过 while self.alive 循环来读取日志队列的。\n\n当子进程在 run_process 中执行完 logger.info("此版本为演示用途") 并执行 return 退出后,self.alive 会立即变为 False。这可能导致 _thread_log_queue_handler 线程在还没来得及从队列中读取最后这条日志时就终止了循环,从而导致 "此版本为演示用途" 无法被追加到 self.renderables 中。\n\n如果发生这种情况,state 将无法匹配到该关键字,进而错误地返回状态 3(异常/警告)而不是 2(未运行)。\n\n建议:\n在 _thread_log_queue_handler 退出 while self.alive 循环后,增加一个清空队列中剩余日志的逻辑。例如:\npython\n# 在 while self.alive 循环结束后\nwhile not self._renderable_queue.empty():\n try:\n log = self._renderable_queue.get_nowait()\n self.renderables.append(log)\n except queue.Empty:\n break\n\n由于 _thread_log_queue_handler 的定义不在当前 diff 中,请在对应位置手动进行修复。
| from module.config.utils import DEFAULT_CONFIG_NAME | ||
|
|
||
|
|
||
| def is_demo_mode(): | ||
| return os.environ.get("DEMO") == "1" | ||
|
|
There was a problem hiding this comment.
在 module/webui/api.py 和 module/webui/app.py 中都重复定义了 is_demo_mode() 函数。此外,在 process_manager.py、app.py(如第 5095 行)以及 config.py 中仍然直接使用了 os.environ.get("DEMO") == "1"。\n\n为了提高代码的可维护性和一致性,建议将 is_demo_mode() 统一移动到公共工具模块(例如 module/config/utils.py,该模块已被这些文件广泛导入),并在所有相关地方统一调用。
| from module.config.utils import DEFAULT_CONFIG_NAME | |
| def is_demo_mode(): | |
| return os.environ.get("DEMO") == "1" | |
| from module.config.utils import DEFAULT_CONFIG_NAME, is_demo_mode |
There was a problem hiding this comment.
Hey - 我发现了两个问题,并提供了一些整体反馈:
- 当前在多个位置(
webui/api.py、webui/app.py、process_manager.py)都实现了演示模式检查,而且使用的模式略有不同;建议将is_demo_mode()(以及DEMO环境变量相关逻辑)集中到一个共享的工具函数中,以避免行为偏差,并让后续修改更安全。 DEMO_DEVICE_ID_TEXT和用于结束演示的日志字符串使用了不同的硬编码文案来表示演示模式;建议为演示识别相关的字符串定义一个统一的常量(或常量集合),以保持 UI 与进程管理器中的检查逻辑一致。
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- 当前在多个位置(`webui/api.py`、`webui/app.py`、`process_manager.py`)都实现了演示模式检查,而且使用的模式略有不同;建议将 `is_demo_mode()`(以及 `DEMO` 环境变量相关逻辑)集中到一个共享的工具函数中,以避免行为偏差,并让后续修改更安全。
- `DEMO_DEVICE_ID_TEXT` 和用于结束演示的日志字符串使用了不同的硬编码文案来表示演示模式;建议为演示识别相关的字符串定义一个统一的常量(或常量集合),以保持 UI 与进程管理器中的检查逻辑一致。
## Individual Comments
### Comment 1
<location path="module/webui/process_manager.py" line_range="183-182" />
<code_context>
if update_tail_hit:
return 4
return 2
+ elif "此版本为演示用途" in s:
+ return 2
elif update_tail_hit:
return 4
</code_context>
<issue_to_address>
**suggestion (bug_risk):** 与特定日志消息字符串的紧密耦合可能会比较脆弱。
这个条件依赖日志中的字面子串 "此版本为演示用途",将行为绑定到一个面向用户的本地化文案上,而这些文案可能会发生变化(文案修改、国际化、重写等)。相较于匹配具体的消息文本,更推荐使用结构化且稳定的信号(例如专门的非本地化标签/前缀或日志中的机器可读标记)。
建议实现方式:
```python
elif DEMO_LOG_MARKER in s:
return 2
```
```python
set_func_logger(func=q.put)
DEMO_LOG_MARKER = "[DEMO_MODE]"
if os.environ.get("DEMO") == "1":
```
`) 要求所有产生与演示相关日志的代码都包含这个标记。
以下是具体修改:
<file_operations>
<file_operation operation="edit" file_path="module/webui/process_manager.py">
<<<<<<< SEARCH
elif "此版本为演示用途" in s:
return 2
=======
elif DEMO_LOG_MARKER in s:
return 2
>>>>>>> REPLACE
<<<<<<< SEARCH
set_func_logger(func=q.put)
if os.environ.get("DEMO") == "1":
=======
set_func_logger(func=q.put)
DEMO_LOG_MARKER = "[DEMO_MODE]"
if os.environ.get("DEMO") == "1":
>>>>>>> REPLACE
</file_operation>
</file_operations>
<additional_changes>
1. 在产生该日志行的地方(原来包含 `"此版本为演示用途"` 的那一行),以一种稳定的方式在日志信息中前置或插入 `DEMO_LOG_MARKER` 字符串 `"[DEMO_MODE]"`(例如:`" [DEMO_MODE] 此版本为演示用途..."`)。
2. 如果存在多条与演示相关的日志消息,建议统一使用同一个标记,以简化消费端逻辑。
3. 如果你更偏好通过配置而不是硬编码标记,后续可以将 `DEMO_LOG_MARKER` 重构为模块级常量或配置项,但仍需保持其非本地化且稳定。
</issue_to_address>
### Comment 2
<location path="module/webui/process_manager.py" line_range="235-242" />
<code_context>
def _alas_start(self):
self.alas.start(None, updater.event)
- if os.environ.get("DEMO") == "1":
- threading.Timer(5, self.alas.stop).start()
</code_context>
<issue_to_address>
**suggestion:** 演示模式检测是临时实现的,而不是通过共享的辅助函数来完成。
其他模块使用 `is_demo_mode()` 辅助函数,而这个文件则直接读取 `os.environ.get("DEMO") == "1"`。为了在将来修改演示模式逻辑时(例如添加额外条件或配置)避免行为出现偏差,请在这里复用共享辅助函数,或者引入一个小的本地辅助函数并委托给它。
建议实现方式:
```python
set_func_logger(func=q.put)
if is_demo_mode():
logger.info("Log3")
time.sleep(1)
logger.info("Log2")
time.sleep(1)
logger.info("Log1")
time.sleep(1)
logger.info("此版本为演示用途")
return
```
1. 确保在 `module/webui/process_manager.py` 顶部从其他模块使用的相同位置导入 `is_demo_mode`(例如 `from module.webui.utils import is_demo_mode` 或等价路径)。
2. 如果该文件中还有其他地方直接读取 `os.environ.get("DEMO")`,也请更新为使用 `is_demo_mode()` 以保持行为一致。
</issue_to_address>帮我变得更有用!请在每条评论上点击 👍 或 👎,我会根据你的反馈改进后续的代码审查。
Original comment in English
Hey - I've found 2 issues, and left some high level feedback:
- The demo-mode check is implemented in multiple places (
webui/api.py,webui/app.py,process_manager.py) with slightly different patterns; consider centralizingis_demo_mode()(and theDEMOenv logic) in a shared utility to avoid drift and make future changes safer. DEMO_DEVICE_ID_TEXTand the demo termination log string use different hardcoded messages to indicate demo mode; consider defining a single shared constant (or constants) for demo-identification strings to keep UI and process-manager checks aligned.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The demo-mode check is implemented in multiple places (`webui/api.py`, `webui/app.py`, `process_manager.py`) with slightly different patterns; consider centralizing `is_demo_mode()` (and the `DEMO` env logic) in a shared utility to avoid drift and make future changes safer.
- `DEMO_DEVICE_ID_TEXT` and the demo termination log string use different hardcoded messages to indicate demo mode; consider defining a single shared constant (or constants) for demo-identification strings to keep UI and process-manager checks aligned.
## Individual Comments
### Comment 1
<location path="module/webui/process_manager.py" line_range="183-182" />
<code_context>
if update_tail_hit:
return 4
return 2
+ elif "此版本为演示用途" in s:
+ return 2
elif update_tail_hit:
return 4
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Tight coupling to a specific log message string may be brittle.
This condition depends on the literal substring "此版本为演示用途" in the log, tying behavior to a localized, human-facing message that may change (copy edits, i18n, rewording). Prefer a structured, stable signal (e.g., a dedicated non-localized tag/prefix or machine-readable marker in the log) instead of matching the exact message text.
Suggested implementation:
```python
elif DEMO_LOG_MARKER in s:
return 2
```
```python
set_func_logger(func=q.put)
DEMO_LOG_MARKER = "[DEMO_MODE]"
if os.environ.get("DEMO") == "1":
```
`) that any demo-related log producers include this marker.
Here are the concrete edits:
<file_operations>
<file_operation operation="edit" file_path="module/webui/process_manager.py">
<<<<<<< SEARCH
elif "此版本为演示用途" in s:
return 2
=======
elif DEMO_LOG_MARKER in s:
return 2
>>>>>>> REPLACE
<<<<<<< SEARCH
set_func_logger(func=q.put)
if os.environ.get("DEMO") == "1":
=======
set_func_logger(func=q.put)
DEMO_LOG_MARKER = "[DEMO_MODE]"
if os.environ.get("DEMO") == "1":
>>>>>>> REPLACE
</file_operation>
</file_operations>
<additional_changes>
1. Wherever this log line is produced (the one that previously contained `"此版本为演示用途"`), prepend or inject the `DEMO_LOG_MARKER` string `"[DEMO_MODE]"` into the log message in a stable way (e.g. `" [DEMO_MODE] 此版本为演示用途..."`).
2. If there are multiple demo-related messages, standardize on using this same marker for all of them to keep the consumer logic simple.
3. If you prefer configuration over a hardcoded marker, you can later refactor `DEMO_LOG_MARKER` into a module-level constant or configuration setting, but keep it non-localized and stable.
</issue_to_address>
### Comment 2
<location path="module/webui/process_manager.py" line_range="235-242" />
<code_context>
def _alas_start(self):
self.alas.start(None, updater.event)
- if os.environ.get("DEMO") == "1":
- threading.Timer(5, self.alas.stop).start()
</code_context>
<issue_to_address>
**suggestion:** Demo-mode detection is implemented ad hoc instead of using a shared helper.
Other modules use an `is_demo_mode()` helper, while this file reads `os.environ.get("DEMO") == "1"` directly. To avoid future behavior drift if demo-mode logic changes (e.g., extra conditions or config), please reuse the shared helper here or introduce a small local helper that delegates to it.
Suggested implementation:
```python
set_func_logger(func=q.put)
if is_demo_mode():
logger.info("Log3")
time.sleep(1)
logger.info("Log2")
time.sleep(1)
logger.info("Log1")
time.sleep(1)
logger.info("此版本为演示用途")
return
```
1. Ensure `is_demo_mode` is imported at the top of `module/webui/process_manager.py` from the same place other modules use (e.g., `from module.webui.utils import is_demo_mode` or equivalent).
2. If there are other direct reads of `os.environ.get("DEMO")` in this file, update them to use `is_demo_mode()` as well to keep behavior consistent.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| @@ -180,6 +180,8 @@ def state(self) -> int: | |||
| if update_tail_hit: | |||
| return 4 | |||
| return 2 | |||
There was a problem hiding this comment.
suggestion (bug_risk): 与特定日志消息字符串的紧密耦合可能会比较脆弱。
这个条件依赖日志中的字面子串 "此版本为演示用途",将行为绑定到一个面向用户的本地化文案上,而这些文案可能会发生变化(文案修改、国际化、重写等)。相较于匹配具体的消息文本,更推荐使用结构化且稳定的信号(例如专门的非本地化标签/前缀或日志中的机器可读标记)。
建议实现方式:
elif DEMO_LOG_MARKER in s:
return 2 set_func_logger(func=q.put)
DEMO_LOG_MARKER = "[DEMO_MODE]"
if os.environ.get("DEMO") == "1":`) 要求所有产生与演示相关日志的代码都包含这个标记。
以下是具体修改:
<file_operations>
<file_operation operation="edit" file_path="module/webui/process_manager.py">
<<<<<<< SEARCH
elif "此版本为演示用途" in s:
return 2
elif DEMO_LOG_MARKER in s:
return 2
REPLACE
<<<<<<< SEARCH
set_func_logger(func=q.put)
if os.environ.get("DEMO") == "1":
=======
set_func_logger(func=q.put)
DEMO_LOG_MARKER = "[DEMO_MODE]"
if os.environ.get("DEMO") == "1":
REPLACE
</file_operation>
</file_operations>
<additional_changes>
- 在产生该日志行的地方(原来包含
"此版本为演示用途"的那一行),以一种稳定的方式在日志信息中前置或插入DEMO_LOG_MARKER字符串"[DEMO_MODE]"(例如:" [DEMO_MODE] 此版本为演示用途...")。 - 如果存在多条与演示相关的日志消息,建议统一使用同一个标记,以简化消费端逻辑。
- 如果你更偏好通过配置而不是硬编码标记,后续可以将
DEMO_LOG_MARKER重构为模块级常量或配置项,但仍需保持其非本地化且稳定。
Original comment in English
suggestion (bug_risk): Tight coupling to a specific log message string may be brittle.
This condition depends on the literal substring "此版本为演示用途" in the log, tying behavior to a localized, human-facing message that may change (copy edits, i18n, rewording). Prefer a structured, stable signal (e.g., a dedicated non-localized tag/prefix or machine-readable marker in the log) instead of matching the exact message text.
Suggested implementation:
elif DEMO_LOG_MARKER in s:
return 2 set_func_logger(func=q.put)
DEMO_LOG_MARKER = "[DEMO_MODE]"
if os.environ.get("DEMO") == "1":`) that any demo-related log producers include this marker.
Here are the concrete edits:
<file_operations>
<file_operation operation="edit" file_path="module/webui/process_manager.py">
<<<<<<< SEARCH
elif "此版本为演示用途" in s:
return 2
elif DEMO_LOG_MARKER in s:
return 2
REPLACE
<<<<<<< SEARCH
set_func_logger(func=q.put)
if os.environ.get("DEMO") == "1":
=======
set_func_logger(func=q.put)
DEMO_LOG_MARKER = "[DEMO_MODE]"
if os.environ.get("DEMO") == "1":
REPLACE
</file_operation>
</file_operations>
<additional_changes>
- Wherever this log line is produced (the one that previously contained
"此版本为演示用途"), prepend or inject theDEMO_LOG_MARKERstring"[DEMO_MODE]"into the log message in a stable way (e.g." [DEMO_MODE] 此版本为演示用途..."). - If there are multiple demo-related messages, standardize on using this same marker for all of them to keep the consumer logic simple.
- If you prefer configuration over a hardcoded marker, you can later refactor
DEMO_LOG_MARKERinto a module-level constant or configuration setting, but keep it non-localized and stable.
| if os.environ.get("DEMO") == "1": | ||
| logger.info("Log3") | ||
| time.sleep(1) | ||
| logger.info("Log2") | ||
| time.sleep(1) | ||
| logger.info("Log1") | ||
| time.sleep(1) | ||
| logger.info("此版本为演示用途") |
There was a problem hiding this comment.
suggestion: 演示模式检测是临时实现的,而不是通过共享的辅助函数来完成。
其他模块使用 is_demo_mode() 辅助函数,而这个文件则直接读取 os.environ.get("DEMO") == "1"。为了在将来修改演示模式逻辑时(例如添加额外条件或配置)避免行为出现偏差,请在这里复用共享辅助函数,或者引入一个小的本地辅助函数并委托给它。
建议实现方式:
set_func_logger(func=q.put)
if is_demo_mode():
logger.info("Log3")
time.sleep(1)
logger.info("Log2")
time.sleep(1)
logger.info("Log1")
time.sleep(1)
logger.info("此版本为演示用途")
return- 确保在
module/webui/process_manager.py顶部从其他模块使用的相同位置导入is_demo_mode(例如from module.webui.utils import is_demo_mode或等价路径)。 - 如果该文件中还有其他地方直接读取
os.environ.get("DEMO"),也请更新为使用is_demo_mode()以保持行为一致。
Original comment in English
suggestion: Demo-mode detection is implemented ad hoc instead of using a shared helper.
Other modules use an is_demo_mode() helper, while this file reads os.environ.get("DEMO") == "1" directly. To avoid future behavior drift if demo-mode logic changes (e.g., extra conditions or config), please reuse the shared helper here or introduce a small local helper that delegates to it.
Suggested implementation:
set_func_logger(func=q.put)
if is_demo_mode():
logger.info("Log3")
time.sleep(1)
logger.info("Log2")
time.sleep(1)
logger.info("Log1")
time.sleep(1)
logger.info("此版本为演示用途")
return- Ensure
is_demo_modeis imported at the top ofmodule/webui/process_manager.pyfrom the same place other modules use (e.g.,from module.webui.utils import is_demo_modeor equivalent). - If there are other direct reads of
os.environ.get("DEMO")in this file, update them to useis_demo_mode()as well to keep behavior consistent.
Summary by Sourcery
添加一个演示模式,用于禁用资源密集型的实时预览/控制功能,并调整标识符和进程以支持安全的演示构建。
新功能:
DEMO=1控制,在演示构建中改变 Web UI 的行为。改进:
Original summary in English
Summary by Sourcery
Add a demo mode that disables resource-intensive live preview/control features and adjusts identifiers and processes to support a safe demonstration build.
New Features:
Enhancements: