feat(render): add configurable long image layouts#87
Merged
Conversation
Co-authored-by: GPT-5 Codex <noreply@openai.com>
Co-authored-by: GPT-5 Codex <noreply@openai.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughChangesLong-image rendering adds Rendering and release update
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Tool
participant resolve_render_layout
participant render_html_to_image
participant Playwright
Tool->>resolve_render_layout: resolve layout, width, and padding
resolve_render_layout-->>Tool: return render kwargs and screenshot style
Tool->>render_html_to_image: render HTML or Markdown with layout kwargs
render_html_to_image->>Playwright: capture screenshot with scale and style
Playwright-->>render_html_to_image: return image
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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: 5
🤖 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 `@src/Undefined/config/load_sections/network.py`:
- Around line 26-28: Add explicit integer type annotations to
_LONG_IMAGE_MIN_WIDTH, _LONG_IMAGE_MAX_WIDTH, and _LONG_IMAGE_MAX_PADDING in the
network configuration module, preserving their existing values and names.
In `@src/Undefined/render.py`:
- Around line 100-130: Move browser-path filesystem discovery out of the event
loop: update _resolve_configured_browser_executable to perform
Path.resolve/is_file through the project async I/O utility (or its
asyncio.to_thread-backed helper), and update _find_system_browser_executable to
run shutil.which through the same utility. Make the callers, including
_get_browser, await these asynchronous helpers before launching Chromium while
preserving configured-path precedence and validation behavior.
In `@tests/test_render_layout_tools.py`:
- Around line 75-81: Update the async test helpers around _fake_render and the
schema-loading code to route all artifact writes and schema reads through the
project utilities in utils/io.py. Replace direct Path.write_bytes and schema
file reads with the established async-safe, atomic I/O helpers, preserving the
existing test data and behavior.
In `@tests/test_render.py`:
- Around line 220-225: Update test_get_browser_uses_configured_executable to
create the executable fixture through the repository’s async-safe helper in
utils/io.py instead of calling Path.write_bytes directly. Preserve the existing
executable path and binary contents while ensuring disk I/O does not block the
event loop.
- Around line 441-484: Update render_html_with_page and its test
test_render_html_with_page_keeps_html_and_javascript_enabled so
caller-controlled HTML is not rendered with unrestricted JavaScript or network
access by default. Add an explicit trusted mode or enforce request
interception/egress blocking for loopback and private-network targets, then
revise the test to assert the secure default rather than requiring JavaScript to
remain enabled.
🪄 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 Plus
Run ID: 2493ad00-7312-4169-90be-930bfb30107e
⛔ Files ignored due to path filters (5)
apps/undefined-chat/package-lock.jsonis excluded by!**/package-lock.jsonapps/undefined-chat/src-tauri/Cargo.lockis excluded by!**/*.lockapps/undefined-console/package-lock.jsonis excluded by!**/package-lock.jsonapps/undefined-console/src-tauri/Cargo.lockis excluded by!**/*.lockuv.lockis excluded by!**/*.lock
📒 Files selected for processing (33)
CHANGELOG.mdapps/undefined-chat/package.jsonapps/undefined-chat/src-tauri/Cargo.tomlapps/undefined-chat/src-tauri/tauri.conf.jsonapps/undefined-console/package.jsonapps/undefined-console/src-tauri/Cargo.tomlapps/undefined-console/src-tauri/tauri.conf.jsonconfig.toml.exampledocs/configuration.mddocs/deployment.mddocs/usage.mdpyproject.tomlsrc/Undefined/__init__.pysrc/Undefined/config/config_class.pysrc/Undefined/config/env_registry.pysrc/Undefined/config/hot_reload.pysrc/Undefined/config/load_sections/network.pysrc/Undefined/render.pysrc/Undefined/skills/toolsets/README.mdsrc/Undefined/skills/toolsets/render/README.mdsrc/Undefined/skills/toolsets/render/layout.pysrc/Undefined/skills/toolsets/render/render_html/config.jsonsrc/Undefined/skills/toolsets/render/render_html/handler.pysrc/Undefined/skills/toolsets/render/render_markdown/config.jsonsrc/Undefined/skills/toolsets/render/render_markdown/handler.pysrc/Undefined/utils/render_cache.pytests/test_config_api.pytests/test_config_env_only.pytests/test_config_env_registry.pytests/test_config_hot_reload.pytests/test_render.pytests/test_render_cache.pytests/test_render_layout_tools.py
Co-authored-by: GPT-5 Codex <noreply@openai.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@src/Undefined/render.py`:
- Around line 406-413: Update the render sandbox networking flow around the
hostname/IP validation guard to prevent DNS rebinding: route browser render
traffic through an egress proxy that resolves and validates destination IPs, or
disable external networking entirely. Do not rely on the current normalized_host
hostname-only check as the security boundary.
🪄 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 Plus
Run ID: 4297e7d5-f7e6-48c5-85a4-ed003df72ea5
📒 Files selected for processing (9)
CHANGELOG.mddocs/configuration.mddocs/usage.mdsrc/Undefined/config/load_sections/network.pysrc/Undefined/render.pysrc/Undefined/skills/toolsets/render/README.mdsrc/Undefined/utils/io.pytests/test_render.pytests/test_render_layout_tools.py
🚧 Files skipped from review as they are similar to previous changes (6)
- src/Undefined/skills/toolsets/render/README.md
- docs/usage.md
- CHANGELOG.md
- src/Undefined/config/load_sections/network.py
- docs/configuration.md
- tests/test_render_layout_tools.py
Co-authored-by: GPT-5 Codex <noreply@openai.com>
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/Undefined/render.py (1)
337-344: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDocument or remove the now-unused
proxyparameter inrender_html_with_page.Since
proxywas removed fromcontext_kwargs, it's no longer used in the function body. It's still passed fromrender_html_to_image(line 329) and documented there (line 290) as retained for call compatibility and cache isolation, butrender_html_with_page's own docstring doesn't mention this. Consider either adding a note to the docstring or removing the parameter if no other callers depend on it.♻️ Optional: document the unused parameter
async def render_html_with_page( html_content: str, callback: Callable[[Page], Awaitable[_RenderResult]], *, viewport_width: int = 1280, timeout_ms: int = 60000, proxy: str | None = None, ) -> _RenderResult: - """在共享浏览器实例中打开 HTML 页面并交给调用方渲染。""" + """在共享浏览器实例中打开 HTML 页面并交给调用方渲染。 + + proxy 参数保留用于调用兼容，离线上下文不会发出网络请求。 + """🤖 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 `@src/Undefined/render.py` around lines 337 - 344, Document the intentionally unused proxy parameter in render_html_with_page, noting that it is retained for call compatibility and cache isolation after removal from context_kwargs. Keep the existing parameter and render_html_to_image call path 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 `@src/Undefined/render.py`:
- Around line 337-344: Document the intentionally unused proxy parameter in
render_html_with_page, noting that it is retained for call compatibility and
cache isolation after removal from context_kwargs. Keep the existing parameter
and render_html_to_image call path unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: e9927819-8748-41ac-9344-0c67d867a798
📒 Files selected for processing (13)
CHANGELOG.mdconfig.toml.exampledocs/build.mddocs/configuration.mddocs/deployment.mddocs/usage.mdsrc/Undefined/render.pysrc/Undefined/skills/toolsets/render/README.mdsrc/Undefined/skills/toolsets/render/render_html/config.jsonsrc/Undefined/skills/toolsets/render/render_latex/config.jsonsrc/Undefined/skills/toolsets/render/render_latex/handler.pytests/test_render.pytests/test_render_latex_tool.py
🚧 Files skipped from review as they are similar to previous changes (4)
- config.toml.example
- src/Undefined/skills/toolsets/render/render_html/config.json
- docs/configuration.md
- tests/test_render.py
Co-authored-by: GPT-5 Codex <noreply@openai.com>
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.
变更摘要
验证
uv run pytest tests/：2524 passed, 1 skippeduv run pytest tests/test_bump_version_script.py tests/test_release_notes_script.py：17 passeduv run ruff check .uv run ruff format --check .uv run mypy .uv build --wheelSummary by CodeRabbit
layout=longrendering for HTML/Markdown single long images with configurable defaults (width/padding) and controls forscreenshot_scale/screenshot_style.browser_executable_pathconfiguration with auto fallback to system Chrome/Chromium and new env overrides for long-image defaults and browser path.data:/blob:resources); LaTeX rendering is local-only with no external fallback.layout=longand the new offline behavior.