feat(screen): 添加 Screen Scope 机制，支持全局/局部 screen 分层管理#2191
Open
ShadowLemoon wants to merge 2 commits into
Open
Conversation
- ScreenInfo 新增 app_id 字段，YAML 中声明所属应用（空=全局，非空=局部） - ScreenContext 新增 enter_scope/exit_scope API，自动从 app_id 推导活跃范围 - screen_utils BFS 匹配适配活跃范围，跳过非活跃 screen 的 OCR/模板匹配 - Application 生命周期自动进入/退出 scope 向后兼容：未设 app_id 时行为不变，渐进迁移无风险
Contributor
📝 WalkthroughWalkthrough本PR设计并实现了Screen Scope机制，将屏幕分为全局屏幕和应用局部屏幕，新增作用域管理API，支持插件屏幕加载，并更新应用生命周期管理以匹配屏幕作用域。 Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/one_dragon/base/operation/application_base.py (1)
68-93:⚠️ Potential issue | 🟠 Major确保异常路径也会退出 screen scope。
当前
enter_scope()只依赖after_operation_done()配对清理；但src/one_dragon/base/operation/operation.py:477的after_operation_done()不在finally中，app.execute()抛出未处理异常时会跳过 Line 93，导致后续应用继续使用上一个应用的活跃 screen 集合。建议把
exit_scope()放到 guaranteed cleanup 路径中，例如在Application.execute()或应用运行外层finally中兜底调用；exit_scope()是幂等的，正常完成时重复调用也安全。一种兜底方向
+ def execute(self) -> OperationResult: + try: + return super().execute() + finally: + self.ctx.screen_loader.exit_scope() + def after_operation_done(self, result: OperationResult): """ 停止后的处理🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/one_dragon/base/operation/application_base.py` around lines 68 - 93, enter_scope/exit_scope pairing is not guaranteed on exceptions because current exit_scope() is only in after_operation_done(); update the control flow so ctx.screen_loader.exit_scope() is invoked from a guaranteed cleanup path (e.g. wrap the whole application run in a try/finally inside Application.execute or immediately after calling ctx.screen_loader.enter_scope(self.app_id) so the finally calls ctx.screen_loader.exit_scope()), leaving the existing after_operation_done() call intact (exit_scope is idempotent) and ensuring any early throws still execute the cleanup; reference ctx.screen_loader.enter_scope, ctx.screen_loader.exit_scope, after_operation_done, and Application.execute when making the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/develop/screen_scope_design.md`:
- Line 23: 在文档中把三处未标记语言的围栏代码块改为使用语言标识 `text`（即把 ``` 改为 ```text），分别针对包含示例框图的
ScreenContext 代码块（关键字 ScreenContext）、包含调用示例的 OneDragonContext.init() 代码块（关键字
OneDragonContext.init() / register_application_factory()）以及展示插件目录结构的
plugins/my_plugin/ 代码块（关键字 plugins/ 或 my_plugin/），以消除 markdownlint MD040 报告。
In `@src/one_dragon/base/screen/screen_loader.py`:
- Around line 182-183: When rebuilding routes in init_screen_route(), clear the
existing route table first so removed screens' keys are deleted; specifically,
reset whatever structure holds routes (e.g. self._screen_routes or
screen_routes) before repopulating in init_screen_route() and do the same in the
other rebuild block around lines 273-280, ensuring get_screen_route() will not
return stale entries for uninstalled screens. Make sure to preserve any needed
defaults, then repopulate from current plugins/screens to rebuild a fresh route
map.
- Around line 374-379: The current logic in ScreenLoader (the block using
_global_screen_names and local_names) incorrectly disables scoping when
_global_screen_names is empty; instead remove the early return that checks
_global_screen_names and always compute local_names from screen_info_list
filtered by app_id, then only return/skip enabling scope if local_names is empty
— i.e., delete or disable the "if not self._global_screen_names: return" check
and keep the subsequent local_names computation so scope is only skipped when
local_names is empty (use symbols: _global_screen_names, screen_info_list,
local_names, app_id).
- Around line 169-176: The loader only checks for duplicate screen_name but not
duplicate screen_id, causing silent overwrites in self._id_2_screen; update the
loading loop (where ScreenInfo(data) is created and appended to
self.screen_info_list and mapped into self.screen_info_map and
self._id_2_screen) to also check if screen_info.screen_id already exists in
self._id_2_screen (or in any existing ScreenInfo) and, if so, log a warning
mentioning the conflicting screen_id and skip adding this ScreenInfo (similar to
the existing screen_name conflict path) to avoid overwriting IDs used by main
config or other plugins.
---
Outside diff comments:
In `@src/one_dragon/base/operation/application_base.py`:
- Around line 68-93: enter_scope/exit_scope pairing is not guaranteed on
exceptions because current exit_scope() is only in after_operation_done();
update the control flow so ctx.screen_loader.exit_scope() is invoked from a
guaranteed cleanup path (e.g. wrap the whole application run in a try/finally
inside Application.execute or immediately after calling
ctx.screen_loader.enter_scope(self.app_id) so the finally calls
ctx.screen_loader.exit_scope()), leaving the existing after_operation_done()
call intact (exit_scope is idempotent) and ensuring any early throws still
execute the cleanup; reference ctx.screen_loader.enter_scope,
ctx.screen_loader.exit_scope, after_operation_done, and Application.execute when
making the change.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 41f059c9-6a99-4e2e-ab6a-20b9da8e04ee
📒 Files selected for processing (6)
docs/develop/screen_scope_design.mdsrc/one_dragon/base/operation/application_base.pysrc/one_dragon/base/operation/one_dragon_context.pysrc/one_dragon/base/screen/screen_info.pysrc/one_dragon/base/screen/screen_loader.pysrc/one_dragon/base/screen/screen_utils.py
Comment on lines
+169
to
+176
| screen_info = ScreenInfo(data) | ||
| if screen_info.screen_name in self.screen_info_map: | ||
| log.warning(f"插件画面名称冲突，已跳过: {screen_info.screen_name}") | ||
| continue | ||
|
|
||
| self.screen_info_list.append(screen_info) | ||
| self.screen_info_map[screen_info.screen_name] = screen_info | ||
| self._id_2_screen[screen_info.screen_id] = screen_info |
Contributor
There was a problem hiding this comment.
同时检查插件 screen_id 冲突。
这里只检查了 screen_name，但随后会写入 _id_2_screen[screen_info.screen_id]；插件若复用已有 screen_id 但换了 screen_name，会静默覆盖主配置/其他插件的 ID 映射，后续保存、删除或按 ID 查找都可能指向错误 screen。
建议修改
screen_info = ScreenInfo(data)
if screen_info.screen_name in self.screen_info_map:
log.warning(f"插件画面名称冲突，已跳过: {screen_info.screen_name}")
continue
+ if screen_info.screen_id in self._id_2_screen:
+ log.warning(f"插件画面ID冲突，已跳过: {screen_info.screen_id}")
+ continue
self.screen_info_list.append(screen_info)
self.screen_info_map[screen_info.screen_name] = screen_info
self._id_2_screen[screen_info.screen_id] = screen_info🧰 Tools
🪛 Ruff (0.15.10)
[warning] 171-171: String contains ambiguous ， (FULLWIDTH COMMA). Did you mean , (COMMA)?
(RUF001)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/one_dragon/base/screen/screen_loader.py` around lines 169 - 176, The
loader only checks for duplicate screen_name but not duplicate screen_id,
causing silent overwrites in self._id_2_screen; update the loading loop (where
ScreenInfo(data) is created and appended to self.screen_info_list and mapped
into self.screen_info_map and self._id_2_screen) to also check if
screen_info.screen_id already exists in self._id_2_screen (or in any existing
ScreenInfo) and, if so, log a warning mentioning the conflicting screen_id and
skip adding this ScreenInfo (similar to the existing screen_name conflict path)
to avoid overwriting IDs used by main config or other plugins.
Comment on lines
+182
to
+183
| if added: | ||
| self.init_screen_route() |
Contributor
There was a problem hiding this comment.
重建路由前先清空旧路由表。
插件刷新后如果某个插件 screen 被移除，init_screen_route() 只覆盖当前 screen 的 key，不会删除旧 key；get_screen_route() 仍可能返回已卸载 screen 的陈旧路径。
建议修改
def init_screen_route(self) -> None:
"""
初始化画面间的跳转路径
:return:
"""
+ self.screen_route_map.clear()
+
# 先对任意两个画面之间做初始化
for screen_1 in self.screen_info_list:
self.screen_route_map[screen_1.screen_name] = {}Also applies to: 273-280
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/one_dragon/base/screen/screen_loader.py` around lines 182 - 183, When
rebuilding routes in init_screen_route(), clear the existing route table first
so removed screens' keys are deleted; specifically, reset whatever structure
holds routes (e.g. self._screen_routes or screen_routes) before repopulating in
init_screen_route() and do the same in the other rebuild block around lines
273-280, ensuring get_screen_route() will not return stale entries for
uninstalled screens. Make sure to preserve any needed defaults, then repopulate
from current plugins/screens to rebuild a fresh route map.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
向后兼容：未设 app_id 时行为不变，渐进迁移无风险
Summary by CodeRabbit
发布说明
文档
新功能
改进