Skip to content

Add input-aware OCR CLI defaults - #52

Draft
Jalendar10 wants to merge 2 commits into
baidu:mainfrom
Jalendar10:codex/pdf-cli-controls
Draft

Add input-aware OCR CLI defaults#52
Jalendar10 wants to merge 2 commits into
baidu:mainfrom
Jalendar10:codex/pdf-cli-controls

Conversation

@Jalendar10

@Jalendar10 Jalendar10 commented Jun 30, 2026

Copy link
Copy Markdown

Summary

  • default batch PDF inference to base image mode with a PDF-specific prompt and 1024 n-gram window
  • add CLI controls for prompt, PDF DPI, n-gram settings, and request timeout
  • reuse one HTTP session per worker thread to reduce local request overhead during concurrent SGLang runs
  • add --skip_existing so interrupted image/PDF jobs can resume without reprocessing non-empty markdown outputs
  • update README batch examples and useful options to match the CLI behavior

Why

The README states that multi-page/PDF inference should use base mode, but the batch PDF example used gundam and infer.py applied the same gundam/128 defaults to all inputs. These changes make the CLI choose safer input-aware defaults while keeping the existing image-directory defaults unchanged.

For large OCR runs, users may also need to restart after partial completion. The new resume mode skips completed outputs, and session reuse avoids recreating HTTP client state for every request.

Validation

  • python3 -m py_compile infer.py
  • python3 infer.py --help
  • parsed image_dir and pdf argument defaults with a lightweight import script
  • verified --skip_existing with a temporary two-image fixture where one non-empty output was skipped

Full model inference was not run locally because it requires starting an SGLang server with the Unlimited-OCR model and GPU resources.

@kushdab

kushdab commented Jul 1, 2026

Copy link
Copy Markdown

Right direction overall -- fixing the PDF default to base mode and ngram_window=1024 aligns with the README recommendation and real multi-page behavior. A few things to address:

Explicit user flags are correctly honored

The if args.image_mode is None / if args.prompt is None / if args.ngram_window is None pattern in parse_args() is the right approach -- it provides better defaults without overriding explicit user choices. Good.

Verify DEFAULT_PDF_PROMPT = "Multi page parsing."

"Multi page parsing." is a reasonable prompt but may not be in the model's training distribution. The standard prompt the model was trained on is "document parsing.". Using a prompt outside the training distribution risks subtly degrading output quality on edge-case pages. Worth testing both on a representative PDF sample before committing to this default. If the maintainers confirm "Multi page parsing." is a trained prompt variant, great -- if not, "document parsing." is the safer default.

--skip_existing may silently preserve looping outputs

The current check (output file exists and is non-empty) passes for files produced by looping pages -- those files can contain 80,000 characters of repeated garbage (see #55) and would be skipped on resume. Suggested heuristic: skip only if file size exceeds a meaningful threshold:

def is_complete_output(path: str) -> bool:
    try:
        return os.path.getsize(path) > 512   # ~50+ tokens of real content
    except OSError:
        return False

This still skips files from normal runs while re-processing files that contain near-empty or clearly corrupt outputs.

Server startup optimization is solid

The build_jobs + filter_jobs pattern that checks --skip_existing before starting the SGLang server is a good optimization -- if everything is already done, no server is spun up at all.

Conflict with PR #50 and PR #29

This PR modifies infer_one() and parse_args(). PR #50 (also open) modifies start_server() and parse_args() to add --attention_backend, --page_size, and --mem_fraction_static. PR #29 modifies start_server() to add --trust-remote-code. All three touch overlapping areas -- coordinate merge order to avoid conflicts, or rebase this PR on top of both.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants