refactor(page_numbers): split scan+prompt from background apply#37
Merged
Conversation
Page-numbers was the last tool in TODO.txt's background-task migration
list. Direct migration was awkward because of the mid-flow
QMessageBox.question that asks the user whether to replace existing
page numbers it detects in the chosen margin band.
Split into two phases:
Phase 1 (main thread):
- Validate input, resolve out_path, gather UI parameters.
- Open the doc once and scan the chosen margin band for existing
numbers. The scan reads only a thin band per target page, so it
stays fast enough not to need its own worker.
- If hits are found, prompt Yes/No/Cancel. Cancel aborts cleanly.
- Bail out with a localized warning when the page-target spec
resolves to zero pages (new tool.page_numbers.no_targets key).
Phase 2 (worker thread):
- Re-open the doc, authenticate if encrypted.
- If replace=True, apply redactions over the existing numbers'
bboxes (carried as plain (x0,y0,x1,y1) tuples — no fitz.Rect
objects leak past the Phase-1 with-block).
- Loop target pages adding new numbers, emit per-page progress,
honour worker.is_cancelled() at every iteration.
- Save with garbage=4, deflate=True.
Add 3 new i18n keys × 8 languages: progress.page_numbers.applying
(dialog label), progress.page_numbers.page (per-page status with
{current}/{total}), tool.page_numbers.no_targets (clearer warning
than the previous silent no-op when the page range matched nothing).
Verified on Ubuntu 26.04 + Py3.14.4 with three smoke-test cases:
clean PDF (no prompt), pre-numbered PDF with user-Yes-replace,
pre-numbered PDF with user-Cancel. All three behave correctly:
on_done fires on the main thread, output appears only on success
paths, the Cancel path produces no output file.
Background-task migration is now complete except for import_pdf.py,
which has 8 paths each shelling into a different third-party lib —
that warrants per-converter review rather than a single mechanical
pass.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.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.
Summary
Migrate
page_numbersfrom synchronous UI-thread work to a two-phase design that survives the mid-flow Yes/No/Cancel prompt about replacing existing page numbers. With this PR, the migration list from feedback_background_tasks.md is complete except forimport_pdf(which has 8 paths each shelling into a different third-party lib and needs per-converter review).How the split works
Phase 1 (main thread) — validate input, scan the chosen margin band for existing numbers (thin band per target page, fast enough to stay on the main thread), prompt Yes/No/Cancel if hits are found. The scanned bboxes are carried forward as plain
(x0,y0,x1,y1)tuples so nofitz.Rectobjects escape thewithblock.Phase 2 (worker thread) — re-open the doc + authenticate, optionally apply redactions over the existing-number bboxes, loop target pages adding new numbers via
page.insert_text, emit per-page progress, save.worker.is_cancelled()is checked at every iteration so the dialog's Cancel button actually aborts.Changes
_runinto pre-flight scan + prompt +_run_backgroundapply.progress.page_numbers.applying(dialog label)progress.page_numbers.page(per-page status with{current}/{total})tool.page_numbers.no_targets(replaces a previous silent no-op when the page range matched zero pages)Test plan
Remaining background-task TODO
After this lands, only
import_pdf.py(8 conversion paths via python-docx / python-pptx / openpyxl / BeautifulSoup / ebooklib) is still synchronous. Each path has its own progress shape and dep set; deferred to per-converter PRs.🤖 Generated with Claude Code