Conversation
There was a problem hiding this comment.
🟡 Changes recommended
The updated pages conversion changes existing v2-bridge behavior without corresponding test updates, and the BabelDOC GUI cleanup appears incomplete (unused model-loader code remains).
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR addresses the GUI “pages” selection bug for the precise (v2) kernel by correcting page index conversion when bridging v1-style requests to v2 CLI args, and removes a large block of unused BabelDOC GUI translation code.
Changes:
- Adjust
v2_bridgeto convert 0-based page index lists into 1-based v2--pagesCLI strings. - Remove the unused
babeldoc_translate_fileimplementation from the GUI codepath.
File summaries
| File | Description |
|---|---|
| pdf2zh/kernel/v2_bridge.py | Fixes v1→v2 pages conversion so precise kernel receives correct --pages values. |
| pdf2zh/gui.py | Deletes dead BabelDOC translation function code from the GUI module. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| # Global setup | ||
| custom_blue = gr.themes.Color( | ||
| c50="#E8F3FF", |
There was a problem hiding this comment.
🟢 Approval recommended
The page-index fix is consistent with existing CLI parsing (0-based internal) and is covered by an updated unit test, and the removed GUI code has no remaining references in the repo.
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 0 new
- Review effort level: Lite
Fixes #713
Changes
pdf2zh_nextCLI strictly expects 1-based page arguments. Updatedv2_bridge.pyto correctly shift the indices by +1 when passing the arguments to the precise kernel.babeldoc_translate_filefunction ingui.py. This logic was deprecated during the migration to theKernelRegistrymechanism in commit 84efe44, and its presence was misleading as translations are now solely executed viakernel.translate().