Repository navigation
Adopt canonical output contract (native-default + per-page + truncation auto-fallback) - #1
Merged
Merged
Conversation
Pre-existing WIP, committed as a clean baseline before the output-contract work.
…-in + truncation auto-fallback - Add output_contract.py: input-relative keying, <stem>/<stem>.md with ## Page N, dual-level metadata.json, exit-code policy - Default whole-PDF (1 call); --per-page / --whole-pdf flags; auto-fallback to per-page on detected truncation - Fixes SYS-01 (basename collisions) and SYS-02 (exit 0 on failure); reconcile README Reference implementation for the OCR CLI fleet output contract (docs/plans/00-output-contract).
…r'-name exclusion) Bump the dep pin to v0.1.1 and replace the hand-rolled dir-scan in get_supported_files (which excluded ANY path component literally named 'ocr') with the shared iter_input_files(scan_root, output_root, suffixes). Discovery now excludes the RESOLVED output root, not a generic name match, so inputs under the user's own .../toolkits/ocr/... tree are processed (was: ZERO files) while the engine never re-ingests its own .md/figure outputs. Callers resolve the output root FIRST and pass it in (_process_directory and the --dry-run path), so the dry run reports exactly what the real run processes.
HIGH (durable failure, non-aborting batch): move validate_file_size INSIDE the try in both PDF paths so an oversized PDF returns a FAILED OCRResult instead of raising out of process_file. Wrap the serial loop and single-file path so a pre-OCRResult exception is recorded status=failed and the batch CONTINUES (one bad file no longer partial-aborts the run with no metadata). HIGH (concurrent persistence): the concurrent catch-all now _persist()s a synthesized FAILED DocMetadata (via _persist_failure) instead of only outcome.add(FAILED), so worker-future and _persist failures leave durable per-doc + root-index metadata. _build_doc_metadata tolerates an unreadable input (sentinel checksum) so the failure is never lost. MEDIUM (page contract): a custom --prompt in the native path now still appends the '## Page N' marker instruction, so a multi-page PDF is no longer silently recorded as ONE page (whole-PDF/auto). MEDIUM (Files-API leak): _upload_file deletes the orphaned remote object when the upload ends in FAILED state (the caller's finally never saw it). Run-config fingerprint: wire v0.1.1 run_fingerprint(model, backend, task, prompt) into every DocMetadata and pass it to RootIndex.is_completed, so a re-run under a different model/mode/prompt reprocesses instead of silently reusing cached output. Skip branches now emit the cached .md path so -q still lists already-done docs; v0.1.1 is_completed re-emits a deleted output (on-disk check).
…uristic) The audit and the PR review both flagged this 102-line module: it is imported nowhere and its substring-based is_retryable_error drifts from the wired OCRProcessor._is_retryable (typed exceptions + httpx status codes). Removing it kills the dead-code/drift hazard a reference impl would propagate.
…+ harness Prove the SYS-02 fixes and adopt the v0.1.1 contract behavior: - oversized file in a serial batch records status=failed AND the other files still process (assert_conforms over the real produced tree); - a worker-future exception in the concurrent path persists durable FAILED per-doc + root metadata; - a model change invalidates the run fingerprint and forces reprocess; - custom --prompt native path still enforces '## Page N' (2-page PDF stays 2); - _upload_file deletes the orphan remote object on upload-FAILED; - oversized PDF/image return FAILED (not raise) — updated the old pytest.raises test to the new durable-failure contract; - conformance via the v0.1.1 harness for an image input (stem+ext keying: scan.png -> scan_png/scan.md; the harness now resolves inline image links); - get_supported_files tests updated to the (directory, output_root) signature, plus a regression test that inputs under an 'ocr'-named tree are discovered. Also fixed pre-existing lint (unused imports, F841, SIM117) so ruff check passes.
Bumps the shared ocr-output-contract pin to v0.1.2 (re-locked uv.lock so
a frozen install/CI runs the same contract) and clears the round-2 review
blockers, using the v0.1.2 helpers.
HIGH SYS-02 (unreadable input aborts the whole batch): the directory
pre-filter and single-file paths now checksum via safe_checksum; a None
result records a durable status=failed for that file and the batch
CONTINUES instead of propagating OSError. The serial loop's stat() size
print is moved inside per-file isolation (the second, narrower instance).
HIGH idempotency fingerprint omitted output-affecting flags: pass
extra={"pdf_mode", "include_images"} (resolved values) to run_fingerprint
so a cross-mode re-run (whole-pdf -> auto, or no-images -> include-images)
reprocesses instead of silently reusing the cached result.
MEDIUM auto-fallback false-positive on blank pages: recover the physical
## Page N marker numbers and pass recovered_page_numbers to is_truncated,
so the v0.1.2 tail-aware logic fires only on a dropped tail, not a blank
interior page. The recovered numbers are also passed to assemble_pages so
a model-skipped page is not silently renumbered under --whole-pdf.
MEDIUM (round-1) Files-API upload could hang forever: bound the PROCESSING
poll with a deadline; on timeout delete the orphan and raise so the
per-file failure path records status=failed.
Tests: unreadable-file-batch-continues (chmod-000 mid-batch + single),
blank-interior-page-no-needless-fallback (+ dropped-tail still falls back),
cross-mode-fingerprint-reprocess (whole-pdf->auto, include-images toggle).
136 passed; ruff format + check clean.
Adopt the v0.1.3 contract (failure_checksum / UNREADABLE_CHECKSUM sentinel). Regenerate uv.lock so a frozen install/CI exercises the same tag the engine is reviewed against (rev=v0.1.3, 52554b9).
…cksum
Round-3 HIGH: the default auto path silently cached incomplete native OCR as
status=completed. Two data-loss modes escaped the contract's tail-aware
is_truncated and are now caught upstream, both forcing the per-page fallback:
1. STOP-with-cut-last-page: max(recovered)==actual_pages keeps the page
signal silent. The native prompt now asks the model to emit a terminal
<!-- OCR-END --> sentinel after the last page; its ABSENCE => truncation.
The sentinel is stripped from the saved markdown body.
2. Markerless multi-page collapse: a multi-page PDF (actual_pages>1) with
ZERO ## Page N markers => truncation (recovered is empty, is_truncated
can't see it).
Also: failure/unreadable records now use the contract's failure_checksum, so a
status=failed entry carries the schema-valid UNREADABLE_CHECKSUM sentinel
instead of the non-conforming 'sha256:unavailable'.
Tests: STOP-with-missing-sentinel -> fallback; markerless-multipage -> fallback;
complete-with-sentinel -> no fallback + sentinel stripped from body; unreadable
record carries UNREADABLE_CHECKSUM and passes assert_conforms. Existing
no-fallback native/conformance tests updated to include the sentinel.
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
Adopts the canonical OCR output contract via the shared
ocr-output-contractpackage (v0.1.0), making this engine's output byte-structure-identical to the rest of the fleet:<input-parent>/ocr/<rel>/<stem>/<stem>.mdwith## Page N, no frontmatter, dual-levelmetadata.json(per-doc + root index, input-relative keyed), and a uniform nonzero-exit-on-failure policy.Adopt canonical output contract (native-default + per-page + truncation auto-fallback).
Test plan
136tests pass (incl. conformance viaocr_output_contract.conformance.assert_conforms), backend/API mocked (no GPU/keys needed in CI).Part of the fleet-wide output-contract rollout (see
../ocr/docs/plans/00-output-contract). Do not merge before review.