Conversation
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
审阅者指南添加顶层 Dockerfile 以支持容器化，修复现有 Docker 镜像路径，并增强 WebUI 的演示模式和直播预览的全屏方向处理。 全屏直播预览方向锁定顺序图sequenceDiagram
actor User
participant WebUI
participant Panel
participant ScreenOrientation
User->>WebUI: enterFullscreenControl()
WebUI->>Panel: requestFullscreen()
Panel-->>WebUI: Promise resolved
WebUI->>WebUI: lockLandscape()
WebUI->>ScreenOrientation: lock('landscape')
User->>WebUI: exitFullscreenControl()
WebUI->>WebUI: unlockOrientation()
WebUI->>ScreenOrientation: unlock()
文件级变更
技巧与命令与 Sourcery 交互
自定义你的体验访问你的 控制面板 以：
获取帮助Original review guide in EnglishReviewer's GuideAdds a top-level Dockerfile for containerization, fixes existing Docker image paths, and enhances WebUI demo mode and fullscreen orientation handling for the live preview. Sequence diagram for fullscreen live preview orientation locksequenceDiagram
actor User
participant WebUI
participant Panel
participant ScreenOrientation
User->>WebUI: enterFullscreenControl()
WebUI->>Panel: requestFullscreen()
Panel-->>WebUI: Promise resolved
WebUI->>WebUI: lockLandscape()
WebUI->>ScreenOrientation: lock('landscape')
User->>WebUI: exitFullscreenControl()
WebUI->>WebUI: unlockOrientation()
WebUI->>ScreenOrientation: unlock()
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces a Dockerfile using uv for dependency management, adds orientation locking to landscape mode when entering fullscreen in the WebUI preview, and implements a demo mode to bypass public WebUI password checks. The reviewer identified several issues: the Dockerfile uses a non-existent Python 3.14 base image and an unsupported '--relocatable' flag for 'uv venv', the CSS changes include an invalid 'orientation' property, and the demo mode logic should return 'None, None' to fully bypass the password prompt.
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.
| # docker build -t hgjazhgj/alas:latest -f deploy/docker/Dockerfile . | ||
| # docker run -v ${PWD}:/app/AzurPilot -p 25548:25548 --name AzurPilot -it --rm hgjazhgj/alas | ||
|
|
||
| FROM python:3.14-slim-bookworm |
Contributor
There was a problem hiding this comment.
Python 3.14 is not yet released as a stable version, and the official python:3.14-slim-bookworm image does not exist on Docker Hub. This will cause the Docker build to fail. Please use a stable Python version such as 3.12 or 3.11.
FROM python:3.12-slim-bookworm
Comment on lines
+48
to
+53
| RUN uv venv --relocatable --python /usr/local/bin/python .venv && \ | ||
| uv sync --frozen --no-dev --no-install-project && \ | ||
| cp /usr/local/bin/uv .venv/bin/uv && \ | ||
| cp /usr/bin/adb .venv/bin/adb && \ | ||
| cp /usr/bin/git .venv/bin/git && \ | ||
| rm -rf /root/.cache/uv |
Contributor
There was a problem hiding this comment.
The uv venv command does not support the --relocatable option, which will cause the Docker build to fail. Additionally, copying the uv, adb, and git binaries into .venv/bin/ duplicates these files and unnecessarily increases the Docker image size. Since these utilities are already in the system PATH, they can be symlinked instead of copied to save space.
RUN uv venv --python /usr/local/bin/python .venv && \
uv sync --frozen --no-dev --no-install-project && \
ln -s /usr/local/bin/uv .venv/bin/uv && \
ln -s /usr/bin/adb .venv/bin/adb && \
ln -s /usr/bin/git .venv/bin/git && \
rm -rf /root/.cache/uv
Comment on lines
258
to
260
| '#alas-live-preview:fullscreen{width:100vw;height:100vh;right:auto;bottom:auto;border:0;border-radius:0;background:#000;}', | ||
| '@media (orientation: portrait){#alas-live-preview:fullscreen{orientation:landscape;}}', | ||
| '#alas-live-preview:fullscreen .alas-live-preview-head{position:absolute;left:0;right:0;top:0;z-index:2;background:rgba(14,19,25,.86);}', |
Contributor
There was a problem hiding this comment.
The CSS property orientation does not exist. Inside a CSS rule, orientation: landscape; is invalid and will be ignored by the browser. Since orientation locking is already handled programmatically via the Screen Orientation API in JavaScript (using screen.orientation.lock), this invalid CSS rule should be removed.
Suggested change
| '#alas-live-preview:fullscreen{width:100vw;height:100vh;right:auto;bottom:auto;border:0;border-radius:0;background:#000;}', | |
| '@media (orientation: portrait){#alas-live-preview:fullscreen{orientation:landscape;}}', | |
| '#alas-live-preview:fullscreen .alas-live-preview-head{position:absolute;left:0;right:0;top:0;z-index:2;background:rgba(14,19,25,.86);}', | |
| '#alas-live-preview:fullscreen{width:100vw;height:100vh;right:auto;bottom:auto;border:0;border-radius:0;background:#000;}', | |
| '#alas-live-preview:fullscreen .alas-live-preview-head{position:absolute;left:0;right:0;top:0;z-index:2;background:rgba(14,19,25,.86);}', |
Comment on lines
+222
to
+223
| if is_demo_mode(): | ||
| return key, None |
Contributor
There was a problem hiding this comment.
In demo mode, the WebUI is typically intended to be publicly accessible without authentication. However, returning key, None means that if a password is set in the configuration or passed via arguments, the application will still prompt visitors for a password. To ensure the demo mode is fully public and bypasses the login screen, consider returning None, None when is_demo_mode() is active.
Suggested change
| if is_demo_mode(): | |
| return key, None | |
| if is_demo_mode(): | |
| return None, None |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - 我发现了 1 个问题，并给出了一些整体性的反馈：
- 新增的 CSS 规则
@media (orientation: portrait){#alas-live-preview:fullscreen{orientation:landscape;}}使用了非标准的orientation属性，大多数浏览器会忽略它；如果你的意图只是进行视觉上的调整，建议改用布局相关属性（例如 width/height/transform），或者完全依赖screen.orientation.lock的逻辑来实现。 - 方向锁只在
exitFullscreenControl()中被释放，因此如果用户通过浏览器自身的 UI（例如 ESC 键或内置控件）退出全屏，而没有经过这条代码路径，unlockOrientation()可能永远不会被调用；建议监听document.onfullscreenchange来检测并处理所有退出全屏的路径。 - 新增的顶层 Dockerfile 中的头部注释仍然引用使用
-f deploy/docker/Dockerfile来构建，这在现在存在两个 Dockerfile 的情况下会让人困惑；建议更新示例命令，明确说明如何使用这个特定的 Dockerfile 进行构建/运行，并将其与deploy/docker/Dockerfile区分开来。
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new CSS rule `@media (orientation: portrait){#alas-live-preview:fullscreen{orientation:landscape;}}` uses a non-standard `orientation` property that most browsers will ignore; if the intent is purely visual adjustment, consider using layout properties (e.g., width/height/transform) or rely solely on the `screen.orientation.lock` logic instead.
- The orientation lock is only released in `exitFullscreenControl()`, so if the user exits fullscreen through the browser UI (e.g., ESC key or built-in controls) without going through this path, `unlockOrientation()` may never run; consider listening to `document.onfullscreenchange` to detect and handle all fullscreen exit paths.
- The header comments in the new top-level Dockerfile still reference building with `-f deploy/docker/Dockerfile`, which is confusing now that there are two Dockerfiles; it would be clearer to update the example commands to show how to build/run using this specific Dockerfile and to distinguish it from `deploy/docker/Dockerfile`.
## Individual Comments
### Comment 1
<location path="Dockerfile" line_range="1-2" />
<code_context>
+# docker build -t hgjazhgj/alas:latest -f deploy/docker/Dockerfile .
+# docker run -v ${PWD}:/app/AzurPilot -p 25548:25548 --name AzurPilot -it --rm hgjazhgj/alas
+
</code_context>
<issue_to_address>
**suggestion:** The build command in the root Dockerfile comment points to the deploy Dockerfile, which is confusing.
Since this Dockerfile is at the repo root, the example build command using `-f deploy/docker/Dockerfile` is ambiguous about which Dockerfile should be used. Please either update the example to reference this file (e.g. `-f Dockerfile`) or explicitly document that this is an alternative to `deploy/docker/Dockerfile` with its own build command.
```suggestion
# Example build using this root Dockerfile:
# docker build -t hgjazhgj/alas:latest -f Dockerfile .
#
# Alternatively, build using the deploy Dockerfile:
# docker build -t hgjazhgj/alas:latest -f deploy/docker/Dockerfile .
#
# Example run:
# docker run -v ${PWD}:/app/AzurPilot -p 25548:25548 --name AzurPilot -it --rm hgjazhgj/alas
```
</issue_to_address>帮我变得更有用！请对每条评论点 👍 或 👎，我会根据你的反馈改进后续的评审。
Original comment in English
Hey - I've found 1 issue, and left some high level feedback:
- The new CSS rule
@media (orientation: portrait){#alas-live-preview:fullscreen{orientation:landscape;}}uses a non-standardorientationproperty that most browsers will ignore; if the intent is purely visual adjustment, consider using layout properties (e.g., width/height/transform) or rely solely on thescreen.orientation.locklogic instead. - The orientation lock is only released in
exitFullscreenControl(), so if the user exits fullscreen through the browser UI (e.g., ESC key or built-in controls) without going through this path,unlockOrientation()may never run; consider listening todocument.onfullscreenchangeto detect and handle all fullscreen exit paths. - The header comments in the new top-level Dockerfile still reference building with
-f deploy/docker/Dockerfile, which is confusing now that there are two Dockerfiles; it would be clearer to update the example commands to show how to build/run using this specific Dockerfile and to distinguish it fromdeploy/docker/Dockerfile.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new CSS rule `@media (orientation: portrait){#alas-live-preview:fullscreen{orientation:landscape;}}` uses a non-standard `orientation` property that most browsers will ignore; if the intent is purely visual adjustment, consider using layout properties (e.g., width/height/transform) or rely solely on the `screen.orientation.lock` logic instead.
- The orientation lock is only released in `exitFullscreenControl()`, so if the user exits fullscreen through the browser UI (e.g., ESC key or built-in controls) without going through this path, `unlockOrientation()` may never run; consider listening to `document.onfullscreenchange` to detect and handle all fullscreen exit paths.
- The header comments in the new top-level Dockerfile still reference building with `-f deploy/docker/Dockerfile`, which is confusing now that there are two Dockerfiles; it would be clearer to update the example commands to show how to build/run using this specific Dockerfile and to distinguish it from `deploy/docker/Dockerfile`.
## Individual Comments
### Comment 1
<location path="Dockerfile" line_range="1-2" />
<code_context>
+# docker build -t hgjazhgj/alas:latest -f deploy/docker/Dockerfile .
+# docker run -v ${PWD}:/app/AzurPilot -p 25548:25548 --name AzurPilot -it --rm hgjazhgj/alas
+
</code_context>
<issue_to_address>
**suggestion:** The build command in the root Dockerfile comment points to the deploy Dockerfile, which is confusing.
Since this Dockerfile is at the repo root, the example build command using `-f deploy/docker/Dockerfile` is ambiguous about which Dockerfile should be used. Please either update the example to reference this file (e.g. `-f Dockerfile`) or explicitly document that this is an alternative to `deploy/docker/Dockerfile` with its own build command.
```suggestion
# Example build using this root Dockerfile:
# docker build -t hgjazhgj/alas:latest -f Dockerfile .
#
# Alternatively, build using the deploy Dockerfile:
# docker build -t hgjazhgj/alas:latest -f deploy/docker/Dockerfile .
#
# Example run:
# docker run -v ${PWD}:/app/AzurPilot -p 25548:25548 --name AzurPilot -it --rm hgjazhgj/alas
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+1
to
+2
| # docker build -t hgjazhgj/alas:latest -f deploy/docker/Dockerfile . | ||
| # docker run -v ${PWD}:/app/AzurPilot -p 25548:25548 --name AzurPilot -it --rm hgjazhgj/alas |
There was a problem hiding this comment.
suggestion: 根目录 Dockerfile 注释中的构建命令指向了 deploy 目录下的 Dockerfile，这会让人困惑。
由于这个 Dockerfile 位于代码库根目录，示例中使用 -f deploy/docker/Dockerfile 的构建命令会让人搞不清到底应该使用哪个 Dockerfile。请考虑将示例更新为引用当前这个文件（例如使用 -f Dockerfile），或者明确说明这是 deploy/docker/Dockerfile 的一个替代方案，并为其单独提供构建命令说明。
Suggested change
| # docker build -t hgjazhgj/alas:latest -f deploy/docker/Dockerfile . | |
| # docker run -v ${PWD}:/app/AzurPilot -p 25548:25548 --name AzurPilot -it --rm hgjazhgj/alas | |
| # Example build using this root Dockerfile: | |
| # docker build -t hgjazhgj/alas:latest -f Dockerfile . | |
| # | |
| # Alternatively, build using the deploy Dockerfile: | |
| # docker build -t hgjazhgj/alas:latest -f deploy/docker/Dockerfile . | |
| # | |
| # Example run: | |
| # docker run -v ${PWD}:/app/AzurPilot -p 25548:25548 --name AzurPilot -it --rm hgjazhgj/alas |
Original comment in English
suggestion: The build command in the root Dockerfile comment points to the deploy Dockerfile, which is confusing.
Since this Dockerfile is at the repo root, the example build command using -f deploy/docker/Dockerfile is ambiguous about which Dockerfile should be used. Please either update the example to reference this file (e.g. -f Dockerfile) or explicitly document that this is an alternative to deploy/docker/Dockerfile with its own build command.
Suggested change
| # docker build -t hgjazhgj/alas:latest -f deploy/docker/Dockerfile . | |
| # docker run -v ${PWD}:/app/AzurPilot -p 25548:25548 --name AzurPilot -it --rm hgjazhgj/alas | |
| # Example build using this root Dockerfile: | |
| # docker build -t hgjazhgj/alas:latest -f Dockerfile . | |
| # | |
| # Alternatively, build using the deploy Dockerfile: | |
| # docker build -t hgjazhgj/alas:latest -f deploy/docker/Dockerfile . | |
| # | |
| # Example run: | |
| # docker run -v ${PWD}:/app/AzurPilot -p 25548:25548 --name AzurPilot -it --rm hgjazhgj/alas |
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.
由 Sourcery 提供的摘要
在演示模式下新增容器化支持并调整 WebUI 行为，同时改进全屏模式下的方向处理。
新功能：
错误修复：
增强改进：
Original summary in English
Summary by Sourcery
Add containerization support and adjust WebUI behavior in demo mode while improving fullscreen orientation handling.
New Features:
Bug Fixes:
Enhancements: