diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index b610f16e..9a87b80d 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -13,9 +13,11 @@ orchestration, MinerU process execution, and DOM normalization. - `src/newsdom_api/service.py` orchestrates PDF parsing, temporary files, and response construction. - `src/newsdom_api/mineru_runner.py` shells out to the MinerU CLI, - collects JSON outputs, and surfaces process errors. + collects JSON outputs, and translates runtime or incomplete-output + failures into typed sanitized exceptions. - `src/newsdom_api/dom_builder.py` converts MinerU `content_list` - blocks into the canonical NewsDOM response model. + blocks plus page model metadata into the canonical NewsDOM response + model. - `src/newsdom_api/schemas.py` defines the public response schema. - `src/newsdom_api/synthetic.py` and `src/newsdom_api/equivalence.py` support synthetic fixture @@ -27,9 +29,13 @@ orchestration, MinerU process execution, and DOM normalization. 2. `src/newsdom_api/service.py` writes the upload to a temporary workspace and calls MinerU. 3. `src/newsdom_api/mineru_runner.py` resolves the executable, runs - the OCR pipeline, and loads generated JSON artifacts. -4. `src/newsdom_api/dom_builder.py` normalizes OCR blocks into the canonical response. -5. FastAPI returns typed JSON from `src/newsdom_api/schemas.py`. + the OCR pipeline, loads generated JSON artifacts, and raises typed + sanitized errors for runtime-unavailable or incomplete-output cases. +4. `src/newsdom_api/dom_builder.py` normalizes OCR blocks into the + canonical response while preserving page-aware structure from + MinerU model metadata. +5. FastAPI returns typed JSON from `src/newsdom_api/schemas.py` and + maps MinerU runtime failures to 503 and incomplete output to 502. ## Supporting systems diff --git a/README.md b/README.md index 5051d107..06032589 100644 --- a/README.md +++ b/README.md @@ -44,14 +44,13 @@ docker build -t newsdom-api . docker run -p 8000:8000 newsdom-api ``` -The default image exposes the REST API on port `8000` as a lean multi-arch -service image. It is suitable for `linux/amd64` and `linux/arm64`, including -Apple Silicon hosts running the API service inside Docker. +The default image exposes the REST API on port `8000` as a multi-arch service +image. It is suitable for `linux/amd64` and `linux/arm64`, including Apple +Silicon hosts running the API service inside Docker. -The lean image is intentionally a REST API shell: `/health`, `/docs`, and -OpenAPI endpoints are available immediately, while real `/parse` execution -still requires a compatible MinerU runtime to be available inside the -container image. +The default image already includes the MinerU runtime and sets +`NEWSDOM_MINERU_BIN=mineru`, so `/parse` is available without layering an extra +OCR package into the container at runtime. For heavier parsing deployments, build the optional NVIDIA-oriented variant: @@ -75,6 +74,12 @@ provide the CUDA user-space/runtime stack required by MinerU. curl -F "file=@sample.pdf" http://127.0.0.1:8000/parse ``` +Each request is written to a request-scoped temporary directory before MinerU +runs, and those temporary files are removed after the response completes. +Sanitized parse failures return `503 MinerU runtime unavailable` when the +runtime cannot be executed and `502 MinerU output was incomplete` when MinerU +finishes without the required output artifacts. + ### Run tests ```bash diff --git a/docs/operations/deploy-runbook.md b/docs/operations/deploy-runbook.md index 4bb403a0..b12a7475 100644 --- a/docs/operations/deploy-runbook.md +++ b/docs/operations/deploy-runbook.md @@ -1,8 +1,9 @@ # Deploy runbook This repository does not ship a long-lived production cluster -definition in-tree, so deployment verification focuses on reproducible -local and GitHub-hosted delivery paths. +definition in-tree and has no in-tree Kubernetes manifests, so +deployment verification focuses on reproducible local and GitHub-hosted +delivery paths. ## Local API smoke @@ -17,7 +18,15 @@ local and GitHub-hosted delivery paths. 1. `docker build -t newsdom-api .` 2. `docker run --rm -p 18080:8000 newsdom-api` -3. `curl -fsS http://127.0.0.1:18080/health` +3. Verify: + - `curl -fsS http://127.0.0.1:18080/health` + - `curl -fsS -F "file=@sample.pdf" http://127.0.0.1:18080/parse` + +The default image already includes the MinerU runtime and sets +`NEWSDOM_MINERU_BIN=mineru`, so container smoke should treat `/parse` +as part of the supported contract instead of a docs-only shell. + +`/health` proves the API process is serving but does not validate a full `/parse` round-trip, MinerU execution, or OCR artifact production. ## Release smoke @@ -30,5 +39,8 @@ local and GitHub-hosted delivery paths. ## Failure handling - Capture sanitized logs outside `tmp/` when a delivery path fails. +- Expect `/parse` failures to stay sanitized: `503 MinerU runtime + unavailable` when the runtime cannot execute, and `502 MinerU output + was incomplete` when required OCR artifacts are missing or invalid. - Reconcile the failure against `README.md`, `CHANGELOG.md`, and the relevant workflow before closing the task. diff --git a/docs/plans/2026-04-23-ocr-accuracy-program-followthrough-design.md b/docs/plans/2026-04-23-ocr-accuracy-program-followthrough-design.md new file mode 100644 index 00000000..0fa5c376 --- /dev/null +++ b/docs/plans/2026-04-23-ocr-accuracy-program-followthrough-design.md @@ -0,0 +1,235 @@ +# Design: OCR accuracy program follow-through on fresh develop baseline + +## Context + +- This worktree is `feature/ocr-accuracy-program-followthrough-3`, created from + the latest `origin/develop` reference (`27d0c05`). +- Fresh baseline verification already passed in this worktree: + - `uv sync --frozen --all-extras` + - `uv run pytest` → `139 passed` +- Current `develop` still has the page-collapse root cause: + - `src/newsdom_api/service.py` forwards only `content_list` + - `src/newsdom_api/dom_builder.py` hardcodes one page and ignores MinerU + `model` metadata + - `src/newsdom_api/schemas.py` exposes richer structures than the builder + actually populates +- Current runtime/docs state is still partially inconsistent: + - `Dockerfile` installs MinerU in the default image + - `README.md` and `manual/api-reference.md` still contain stale runtime/run + contract wording + - deploy docs verify `/health` but do not meaningfully exercise `/parse` +- Current GitHub OCR program already has a canonical hierarchy and milestone: + - milestone `v0.2.0 OCR accuracy and runtime readiness` + - meta issue `#61` + - sub-issues `#60`, `#62`, `#63`, `#64` +- Current open PR topology relevant to this work: + - `#57` MinerU/mkdocs-material/pyinstaller dependency bump, milestone-linked but + not linked to `#60-#64` in body/title + - `#58` GitHub Actions dependency bump, no milestone/issue linkage + - `#51` pypdf dependency bump, no milestone/issue linkage +- Local-only private OCR inputs exist at + `/Users/seonghobae/Documents/newsdom-api/tests/copyrighted_fixtures_for_realities` + as five flat PDF files with no adjacent redacted outputs or sidecars. + +## Constraints + +- Never commit copyrighted source PDFs, OCR text, rendered crops, or secrets. +- Durable evidence must be redacted and structural only: counts, warning types, + status, sanitized summaries, and synthetic fixtures are allowed. +- Repository-local policy requires: + - branch from `develop`, target `develop` + - TDD for every behavior change + - explicit GitHub issue/PR/workflow state checks before acting +- Acceptance still requires: + - `uv run pytest` + - `uv run pytest --cov=src/newsdom_api --cov-branch --cov-report=term-missing --cov-fail-under=100` + - `uv run mkdocs build --strict` when user-facing docs change + - targeted smoke checks for changed delivery paths +- API hardening remains in scope because `/parse` is an externally exposed OCR + path: + - do not leak private paths or raw subprocess errors + - document and test sanitized failure behavior + - keep abuse/rate/resource implications visible even if not fully solved in + this slice +- Kubernetes/runtime concerns stay in scope for docs, smoke evidence, and deploy + readiness even if the first implementation slice is application-level. +- Non-interactive default applies: do not stop for human review; use robot review + and recorded evidence instead. + +## Approaches considered + +### 1. Start with private-fixture runtime debugging + +- Pros: + - immediately attacks the real-document runtime uncertainty + - may surface host/container/runtime blockers early +- Cons: + - leaves the already-proven page-collapse bug unfixed + - depends on environment-specific runtime state before the deterministic code + root cause is repaired + - makes later benchmark interpretation noisier +- Verdict: useful follow-up, not the first implementation slice. + +### 2. Restore page-aware normalization first, then sanitize runtime and docs (recommended) + +- Pros: + - fixes the known root cause described by `#62` + - keeps the first change set deterministic and testable + - allows real-document evidence and engine comparisons to run against the right + DOM contract + - fits the existing issue hierarchy in `#61` +- Cons: + - runtime-safe failure handling and docs drift still need additional steps + - requires schema/doc updates to keep the public contract honest +- Verdict: recommended. + +### 3. Bundle normalization, runtime failure mapping, docs reconciliation, and benchmark workflow in one large change + +- Pros: + - one branch can narrate the whole OCR/runtime program + - fewer rebases between related tasks +- Cons: + - larger review and verification blast radius + - mixes deterministic unit-testable work with environment-sensitive + investigation and GitHub topology maintenance +- Verdict: acceptable as a branch-level sequence, not as one undivided task. + +## Recommended design + +Execute the program in five layers: + +1. **Restore page-aware NewsDOM normalization (`#62`)** +2. **Add sanitized runtime/output failure handling for `/parse`** +3. **Reconcile runtime/manual/Kubernetes contract drift (`#64`)** +4. **Capture redacted local-only OCR evidence and benchmark prerequisites (`#60`, `#63`)** +5. **Refresh GitHub issue/PR continuity for adjacent open PRs and follow-up tasks** + +### Layer 1: Page-aware normalization contract + +- Update `src/newsdom_api/service.py` so `build_dom(...)` receives both + `content_list` and `model`. +- Update `src/newsdom_api/dom_builder.py` to: + - group blocks by `page_idx` + - use `model[*].page_info.page_no`, width, and height when present + - preserve deterministic page order + - keep `header`, `footer`, `page_number`, and `ad` as page-scoped metadata + - preserve caption/footnote/media metadata for image/table/chart blocks + - emit quality warnings when page assignment is ambiguous or content/model + structure diverges +- Expand `src/newsdom_api/schemas.py` only as far as required to represent the + preserved data truthfully. + +### Layer 2: Sanitized runtime failure handling + +- Introduce typed MinerU runtime/output exceptions. +- `src/newsdom_api/mineru_runner.py` should wrap: + - missing executable failures + - `subprocess.CalledProcessError` + - incomplete output artifact cases +- `src/newsdom_api/main.py` should map them to stable sanitized API responses: + - `503` for runtime/unavailable parser failures + - `502` for incomplete parser outputs +- Keep private file paths and raw stderr out of public responses. + +### Layer 3: Runtime/manual/Kubernetes contract reconciliation + +- Align these truth sources with the actual code path: + - `README.md` + - `manual/api-reference.md` + - `docs/operations/deploy-runbook.md` + - any touched architecture/agent docs when the runtime contract changes +- Document default image behavior, temp-storage expectations, `/health`, and any + relevant CPU/GPU or multi-arch delivery caveats. +- Add or extend smoke evidence so `/parse` contract drift is harder to miss. + +### Layer 4: Local-only OCR evidence and benchmark prerequisites + +- Run at least one private PDF through the updated branch. +- Store only redacted durable evidence: + - page counts + - block counts + - success/failure state + - warning categories + - sanitized runtime summary if failure remains +- Use that evidence to support the next actions for `#60` and `#63`. + +#### 2026-04-23 local-only evidence snapshot + +- Input class: single private PDF fixture from + `/Users/seonghobae/Documents/newsdom-api/tests/copyrighted_fixtures_for_realities` + (`content_file_3998.pdf`) +- Structural source evidence: + - page count: 16 + - PDF encryption: yes +- Current-branch local-only parse result: + - state: failure + - block count: unavailable because MinerU runtime did not start and no DOM was + produced + - warning categories: none observed before failure + - sanitized runtime summary: `MinerU runtime unavailable` +- Readiness implication: + - `#60` remains blocked on a host or container environment that actually has + the MinerU executable available for private-fixture benchmarking + - `#63` can still use this snapshot as proof that the remaining gap is runtime + availability rather than additional unredacted output handling + +### Layer 5: GitHub topology continuity + +- Preserve `#61` as the O1 canonical program anchor. +- Keep `#60`, `#62`, `#63`, `#64` as the active O2 task set. +- Treat open PRs as follows: + - `#57`: related OCR/runtime follow-up PR; keep milestone and add explicit + issue linkage if missing + - `#58`: delivery-governance follow-up; classify as adjacent/non-core + - `#51`: PDF-dependency follow-up; classify as adjacent and only milestone-link + if current-code evidence justifies it +- Do not create duplicate PRs for the same branch; use PR continuity before + opening a new PR. + +## Data flow + +1. `POST /parse` receives an uploaded PDF. +2. `src/newsdom_api/service.py` writes a sanitized temporary filename. +3. `src/newsdom_api/mineru_runner.py` invokes MinerU and returns structured data + or raises a typed sanitized exception. +4. `src/newsdom_api/dom_builder.py` constructs a multi-page NewsDOM response from + `content_list` plus `model`. +5. `src/newsdom_api.main` returns a `ParseResponse` or a stable sanitized HTTP + error. +6. Local-only private-fixture reruns produce only redacted structural evidence. + +## Error handling + +- Parser runtime failures must not leak temp paths or raw subprocess details. +- Incomplete MinerU output is a typed backend failure, not a raw file error. +- Page-structure mismatches that still permit normalization belong in + `ParseQuality.warnings`, not HTTP errors. +- Stale docs that overpromise schema/runtime behavior are acceptance failures. + +## Testing strategy + +- **TDD layer 1** + - failing synthetic multi-page regressions in `tests/test_dom_builder.py` and + `tests/test_service.py` +- **TDD layer 2** + - failing runner/error-path regressions in `tests/test_mineru_runner_paths.py` + and parse endpoint tests +- **Docs/runtime alignment** + - existing docs/manual/governance tests where applicable + - `uv run mkdocs build --strict` +- **Repository gates** + - `uv run pytest` + - `uv run pytest --cov=src/newsdom_api --cov-branch --cov-report=term-missing --cov-fail-under=100` +- **Targeted smokes** + - parser-related smoke for changed delivery paths + - local-only private fixture rerun with redacted evidence only + +## Decisions + +- Prioritize the deterministic `#62` root-cause fix before broader OCR engine or + environment benchmarking. +- Keep runtime failures explicit and sanitized rather than silent or generic. +- Treat docs/Kubernetes/runtime reconciliation as part of the OCR delivery path, + not optional cleanup. +- Preserve the existing `#61` hierarchy and repair adjacent PR linkage rather + than reshaping the issue tree unnecessarily. diff --git a/docs/plans/2026-04-23-ocr-accuracy-program-followthrough.md b/docs/plans/2026-04-23-ocr-accuracy-program-followthrough.md new file mode 100644 index 00000000..aea045de --- /dev/null +++ b/docs/plans/2026-04-23-ocr-accuracy-program-followthrough.md @@ -0,0 +1,458 @@ +# OCR Accuracy Program Follow-through Implementation Plan + +> **For Claude:** REQUIRED SUB-SKILL: Use superpowers:executing-plans to implement this plan task-by-task. + +**Goal:** Restore page-aware MinerU structure on the fresh `develop` baseline, add sanitized `/parse` runtime failure handling, reconcile runtime/Kubernetes/manual drift, and record redacted local-only OCR evidence while keeping the GitHub OCR issue/PR program coherent. + +**Architecture:** First add failing synthetic regressions for the page-collapse root cause and implement the minimal schema/service/builder changes needed to pass them. Then add failing runtime-error regressions that require typed MinerU failure handling and sanitized `502`/`503` API responses, followed by docs/runtime reconciliation, local-only redacted evidence collection, and GitHub issue/PR continuity updates for adjacent follow-up work. + +**Tech Stack:** FastAPI, Pydantic, pytest, uv, MinerU CLI, MkDocs, GitHub issues/PRs/milestones + +--- + +### Task 1: Add failing page-aware normalization regressions + +**Files:** +- Modify: `tests/test_dom_builder.py` +- Modify: `tests/test_service.py` +- Create: `tests/fixtures/mineru_multi_page_sample.json` +- Create: `tests/fixtures/mineru_multi_page_model.json` + +**Step 1: Write the failing test** + +Add a new regression in `tests/test_dom_builder.py` that proves: + +- `build_dom(...)` emits multiple pages from a synthetic multi-page fixture +- page width/height come from `model[*].page_info` +- `header`, `footer`, and `page_number` stay at page scope +- `chart` blocks remain preserved media +- image/table/chart caption and footnote metadata survive normalization +- blocks with missing `page_idx` create warnings instead of silent wrong + assignment + +Also update `tests/test_service.py` so `parse_pdf_bytes(...)` must pass both +`content_list` and `model` to `build_dom(...)`. + +**Step 2: Create the synthetic fixtures** + +Create: + +- `tests/fixtures/mineru_multi_page_sample.json` +- `tests/fixtures/mineru_multi_page_model.json` + +Include exactly enough fixture structure to cover: + +- two pages (`0`, `1`) +- header/footer/page-number per page +- image + chart + table metadata with caption/footnote cases +- page-info width/height values + +**Step 3: Run test to verify it fails** + +Run: + +```bash +uv run pytest tests/test_dom_builder.py tests/test_service.py -q +``` + +Expected: FAIL because the current builder collapses everything into one page +and the current service discards `model`. + +**Step 4: Re-run the exact failing cases** + +Run: + +```bash +uv run pytest tests/test_dom_builder.py::test_build_dom_preserves_multi_page_structure_and_page_scoped_metadata tests/test_service.py::test_parse_pdf_bytes_writes_temp_file_and_builds_dom -q +``` + +Expected: FAIL for the intended page-aware behavior gap. + +**Step 5: Commit** + +```bash +git add tests/test_dom_builder.py tests/test_service.py tests/fixtures/mineru_multi_page_sample.json tests/fixtures/mineru_multi_page_model.json +git commit -m "test: add page-aware MinerU regressions" +``` + +### Task 2: Implement the page-aware `#62` core + +**Files:** +- Modify: `src/newsdom_api/dom_builder.py` +- Modify: `src/newsdom_api/service.py` +- Modify: `src/newsdom_api/schemas.py` + +**Step 1: Write the minimal implementation** + +Update the code so that: + +- `build_dom(...)` accepts `model: list[dict[str, Any]] | None = None` +- page indices are derived from `content_list.page_idx` and + `model[*].page_info.page_no` +- pages are emitted in deterministic sorted order +- page width/height are preserved +- headers, footers, page numbers, and ads remain page-scoped +- chart/image/table caption and footnote metadata are preserved +- `ParseQuality.warnings` is populated for ambiguous page assignment or + content/model divergence +- `parse_pdf_bytes(...)` forwards `mineru_output.get("model")` + +**Step 2: Run test to verify it passes** + +Run: + +```bash +uv run pytest tests/test_dom_builder.py tests/test_service.py -q +``` + +Expected: PASS. + +**Step 3: Run the broader parser subset** + +Run: + +```bash +uv run pytest tests/test_dom_builder.py tests/test_service.py tests/test_mineru_runner_paths.py tests/test_parse_endpoint.py tests/test_parse_endpoint_success.py -q +``` + +Expected: PASS. + +**Step 4: Run targeted coverage on touched parser modules** + +Run: + +```bash +uv run pytest tests/test_dom_builder.py tests/test_service.py tests/test_mineru_runner_paths.py tests/test_parse_endpoint.py tests/test_parse_endpoint_success.py --cov=src/newsdom_api.dom_builder --cov=src/newsdom_api.service --cov=src/newsdom_api.schemas --cov-branch --cov-report=term-missing +``` + +Expected: PASS with strong signal for the touched parser modules. + +**Step 5: Commit** + +```bash +git add src/newsdom_api/dom_builder.py src/newsdom_api/service.py src/newsdom_api/schemas.py +git commit -m "feat: preserve multi-page MinerU structure in NewsDOM" +``` + +### Task 3: Add failing runtime-safe `/parse` regressions + +**Files:** +- Modify: `tests/test_mineru_runner_paths.py` +- Modify: `tests/test_parse_endpoint.py` +- Modify: `tests/test_parse_endpoint_success.py` + +**Step 1: Write the failing runner tests** + +Extend `tests/test_mineru_runner_paths.py` so it requires: + +- `run_mineru(...)` wraps `CalledProcessError` in a typed sanitized runtime + exception +- missing executable failures are wrapped in the same runtime-unavailable class +- missing `ocr/`, `*_content_list.json`, or `*_model.json` become a typed + incomplete-output exception +- runtime exceptions retain internal debug metadata (`returncode`, `stdout`, + `stderr`) without exposing unsafe public strings + +**Step 2: Write the failing API tests** + +Extend parse endpoint tests so `/parse` returns: + +- `503` with sanitized detail when MinerU is unavailable or crashes +- `502` with sanitized detail when MinerU outputs are incomplete + +Also assert that temp directories and private-looking paths are absent from the +public detail string. + +**Step 3: Run test to verify it fails** + +Run: + +```bash +uv run pytest tests/test_mineru_runner_paths.py tests/test_parse_endpoint.py tests/test_parse_endpoint_success.py -q +``` + +Expected: FAIL because the current code still raises raw subprocess/file +exceptions. + +**Step 4: Re-run the exact new failures** + +Run: + +```bash +uv run pytest tests/test_mineru_runner_paths.py::test_run_mineru_wraps_called_process_error tests/test_parse_endpoint.py::test_parse_endpoint_returns_503_for_mineru_runtime_failure tests/test_parse_endpoint.py::test_parse_endpoint_returns_502_for_incomplete_mineru_output -q +``` + +Expected: FAIL for the intended runtime translation gap. + +**Step 5: Commit** + +```bash +git add tests/test_mineru_runner_paths.py tests/test_parse_endpoint.py tests/test_parse_endpoint_success.py +git commit -m "test: add MinerU runtime failure regressions" +``` + +### Task 4: Implement typed MinerU runtime/output errors and API mapping + +**Files:** +- Create: `src/newsdom_api/errors.py` +- Modify: `src/newsdom_api/mineru_runner.py` +- Modify: `src/newsdom_api/main.py` +- Modify: `src/newsdom_api/service.py` +- Modify: `ARCHITECTURE.md` + +**Step 1: Write the minimal implementation** + +Create explicit exception types for: + +- runtime/unavailable MinerU execution failure +- incomplete MinerU output artifacts + +**Step 2: Update runner and endpoint wiring** + +Change `src/newsdom_api/mineru_runner.py` so it: + +- wraps `CalledProcessError` and missing executable failures in the runtime + exception +- raises the incomplete-output exception when required MinerU artifacts are + absent +- avoids path-bearing public exception strings + +Change `src/newsdom_api/main.py` so `/parse` maps exceptions to: + +- `503 Service Unavailable` +- `502 Bad Gateway` + +Keep `src/newsdom_api/service.py` changes minimal and focused on exception flow +only if required. + +Update `ARCHITECTURE.md` to document page-aware normalization plus typed runtime +failure handling. + +**Step 3: Run test to verify it passes** + +Run: + +```bash +uv run pytest tests/test_mineru_runner_paths.py tests/test_parse_endpoint.py tests/test_parse_endpoint_success.py -q +``` + +Expected: PASS. + +**Step 4: Run the parser regression subset again** + +Run: + +```bash +uv run pytest tests/test_dom_builder.py tests/test_service.py tests/test_mineru_runner_paths.py tests/test_parse_endpoint.py tests/test_parse_endpoint_success.py -q +``` + +Expected: PASS. + +**Step 5: Commit** + +```bash +git add src/newsdom_api/errors.py src/newsdom_api/mineru_runner.py src/newsdom_api/main.py src/newsdom_api/service.py ARCHITECTURE.md +git commit -m "fix: sanitize MinerU runtime failures" +``` + +### Task 5: Reconcile runtime/manual/Kubernetes contract drift + +**Files:** +- Modify: `README.md` +- Modify: `manual/api-reference.md` +- Modify: `docs/operations/deploy-runbook.md` +- Modify: `AGENTS.md` (only if canonical execution guidance materially changes) +- Modify: `ARCHITECTURE.md` (if needed beyond Task 4) + +**Step 1: Write the failing documentation expectations** + +Add or extend the narrowest existing doc/manual/governance tests so they require +the docs to match the actual runtime contract: + +- default image includes MinerU runtime path +- `/parse` failure semantics are described honestly +- temp-file handling and `/health` expectations are current +- Kubernetes/runtime considerations match the current delivery path + +**Step 2: Run test to verify it fails** + +Run the smallest relevant existing test file(s) for the changed docs. + +Expected: FAIL until stale wording is removed. + +**Step 3: Write the minimal doc updates** + +Align all touched docs with the actual code and delivery contract. + +**Step 4: Run docs verification** + +Run: + +```bash +uv run mkdocs build --strict +``` + +Expected: PASS. + +**Step 5: Commit** + +```bash +git add README.md manual/api-reference.md docs/operations/deploy-runbook.md AGENTS.md ARCHITECTURE.md +git commit -m "docs: align OCR runtime contract" +``` + +### Task 6: Record redacted local-only OCR evidence + +**Files:** +- Modify: `docs/plans/2026-04-23-ocr-accuracy-program-followthrough-design.md` +- Modify: `docs/plans/2026-04-23-ocr-accuracy-program-followthrough.md` +- Modify: issue/PR comments only when durable blocker evidence is required + +**Step 1: Run a local-only private-fixture check** + +Use one PDF from `/Users/seonghobae/Documents/newsdom-api/tests/copyrighted_fixtures_for_realities` and capture only: + +- page count +- block count +- success/failure state +- warning categories +- sanitized runtime summary + +Do not store raw OCR output or extracted text in the repo. + +**Step 2: Record only redacted durable evidence** + +Append the redacted evidence summary to the plan/design docs or another durable +repo doc using counts/categories only. + +Recorded 2026-04-23 local-only evidence: + +- input fixture: `content_file_3998.pdf` +- page count: 16 +- block count: unavailable because the parser did not produce a DOM +- success/failure state: failure +- warning categories: none observed before failure +- sanitized runtime summary: `MinerU runtime unavailable` + +**Step 3: Re-check follow-up issue readiness** + +Re-evaluate `#60`, `#63`, and `#64` against the new evidence and current code. +If external blockers remain, record them in issue/PR comments with sanitized +evidence. + +Current local-only readiness readout: + +- `#60`: still externally blocked for real-document benchmarking until a local or + container runtime with the MinerU executable is available +- `#63`: runtime-sanitization work remains evidenced as correct for the observed + failure path because the branch surfaced only `MinerU runtime unavailable` +- `#64`: no new docs blocker from this snapshot; the remaining blocker is runtime + availability, not public-contract wording + +**Step 4: Commit** + +```bash +git add docs/plans/2026-04-23-ocr-accuracy-program-followthrough-design.md docs/plans/2026-04-23-ocr-accuracy-program-followthrough.md +git commit -m "docs: record redacted OCR follow-through evidence" +``` + +### Task 7: Repair GitHub issue/PR continuity and follow-up wiring + +**Files:** +- Modify: GitHub issue/PR metadata and comments only + +**Step 1: Re-check live GitHub state** + +Verify: + +- `#61` sub-issue tree remains intact +- milestone membership for `#60-#64` +- open PRs `#57`, `#58`, `#51` for missing milestone or issue linkage +- current branch PR continuity before creating a new PR + +**Step 2: Apply the minimal metadata fixes** + +Examples: + +- add/update issue references in related PR bodies/comments +- attach milestone when current-code evidence justifies it +- create only genuinely missing follow-up/blocker issues, rooted under `#61` +- keep adjacent non-core PRs as related follow-ups, not duplicates + +**Step 3: Draft or update the canonical PR** + +Create a draft PR for this branch if none exists, using the issue program as the +summary spine. Ensure the PR body references `#61` and the exact implemented +issue(s). + +**Step 4: Re-check continuity after creation/update** + +Run PR continuity again and confirm there is one canonical PR for the branch. + +**Step 5: Commit** + +No git commit for metadata-only GitHub changes. + +### Task 8: Run full acceptance verification and continue delivery + +**Files:** +- Modify: only files required by verification feedback + +**Step 1: Run the full test suite** + +Run: + +```bash +uv run pytest +``` + +Expected: PASS. + +**Step 2: Run the repository coverage gate** + +Run: + +```bash +uv run pytest --cov=src/newsdom_api --cov-branch --cov-report=term-missing --cov-fail-under=100 +``` + +Expected: PASS with 100% coverage. + +**Step 3: Re-run docs build if public docs changed** + +Run: + +```bash +uv run mkdocs build --strict +``` + +Expected: PASS. + +**Step 4: Run targeted delivery smoke checks** + +Run the applicable smokes for changed parser/delivery paths, for example: + +```bash +uv run python fuzzers/dom_builder_fuzzer.py --smoke tests/fixtures/mineru_sample.json +``` + +Add container/runtime smokes if those paths changed. + +**Step 5: Continue PR/merge/deploy follow-through** + +After fresh verification, continue through: + +- pushing the branch +- draft PR → ready-for-review transition when evidence is complete +- robot-review evidence collection +- merge path execution when checks and current-head review evidence allow it +- deploy/release verification or durable blocker capture +- follow-up issue execution if a separate adjacent task becomes the next highest + executable item + +**Step 6: Commit verification-driven fixes** + +```bash +git add +git commit -m "fix: address OCR follow-through verification feedback" +``` diff --git a/manual/api-reference.md b/manual/api-reference.md index aa0b9ce4..482b0c3f 100644 --- a/manual/api-reference.md +++ b/manual/api-reference.md @@ -4,11 +4,11 @@ FastAPI 서버를 실행하고, 스캔된 신문 PDF를 업로드하여 기사 D ## 1. 서버 실행하기 -가상환경을 활성화한 상태에서, `uvicorn`을 이용하여 API 서버(`src/newsdom_api/main.py`)를 구동합니다. +가상환경을 활성화한 상태에서, `uv run uvicorn`으로 API 서버(`src/newsdom_api/main.py`)를 구동합니다. ```bash # 개발 시 핫-리로딩(--reload) 모드 사용 -uvicorn newsdom_api.main:app --reload +uv run uvicorn --app-dir src newsdom_api.main:app --reload ``` ## 2. 인터랙티브 웹 UI @@ -27,7 +27,7 @@ FastAPI는 OpenAPI 기반의 대화형 API 문서를 자동으로 생성합니 ### `POST /parse` -스캔된 일본어 신문 PDF 문서를 업로드 받아, 임시 디렉토리에 저장 후 `mineru` 파이프라인을 백그라운드로 실행하고 변환 결과를 기사 단위 DOM 구조가 담긴 JSON 형태로 반환합니다. +스캔된 일본어 신문 PDF 문서를 업로드 받아, 요청별 임시 디렉토리에 저장한 뒤 `mineru` 파이프라인을 실행하고 변환 결과를 기사 단위 DOM 구조가 담긴 JSON 형태로 반환합니다. 업로드된 PDF와 MinerU 산출물은 응답 후 정리됩니다. #### 요청 매개변수 (Request Body) - **`file`** (`UploadFile`, 필수): 변환할 PDF 바이너리 파일 데이터 (`multipart/form-data`) @@ -66,6 +66,15 @@ else: print(f"에러 발생: {response.status_code}, {response.text}") ``` +#### 실패 응답 계약 + +- `503 Service Unavailable`: `MinerU runtime unavailable` + - `mineru` 실행 파일을 찾을 수 없거나 실행 자체가 실패했을 때 반환됩니다. +- `502 Bad Gateway`: `MinerU output was incomplete` + - MinerU 프로세스는 종료되었지만 필수 JSON 산출물이 없거나 불완전할 때 반환됩니다. + +서버는 내부 `stdout`/`stderr`를 그대로 노출하지 않고 위의 고정된 상세 메시지로만 `/parse` 실패를 응답합니다. + --- ### 응답(Response) JSON 스키마 (`src/newsdom_api/schemas.py`) @@ -144,7 +153,7 @@ else: ### `GET /health` -API 서버의 작동 여부를 검사하는 경량 상태 체크 엔드포인트입니다. 로드밸런서나 쿠버네티스 프로브 등에서 서비스가 살아있는지 검사할 때 사용합니다. +API 서버의 작동 여부를 검사하는 경량 상태 체크 엔드포인트입니다. 로드밸런서나 컨테이너/오케스트레이션 프로브에서 프로세스 응답 여부를 확인할 때 사용합니다. 이 응답은 애플리케이션이 요청을 받고 있음을 보여주지만, MinerU 기반 `/parse` 전체 경로를 보증하지는 않습니다. #### 요청 / 응답 diff --git a/src/newsdom_api/dom_builder.py b/src/newsdom_api/dom_builder.py index 4ca20991..426da8a7 100644 --- a/src/newsdom_api/dom_builder.py +++ b/src/newsdom_api/dom_builder.py @@ -29,6 +29,46 @@ def _bbox_from_values(values: list[float] | None) -> BoundingBox | None: ) +def _coerce_page_number(value: Any) -> int | None: + """Convert supported page-number values into integers.""" + + if value is None or isinstance(value, bool): + return None + try: + return int(value) + except (TypeError, ValueError): + return None + + +def _caption_nodes_from_items(items: Any) -> list[CaptionNode]: + """Normalize caption-like payloads into caption nodes.""" + + nodes: list[CaptionNode] = [] + if not isinstance(items, list): + return nodes + + for item in items: + if isinstance(item, dict): + text = str(item.get("text") or item.get("contents") or "").strip() + bbox = _bbox_from_values(item.get("bbox") or item.get("box")) + else: + text = str(item).strip() + bbox = None + if text: + nodes.append(CaptionNode(text=text, bbox=bbox)) + return nodes + + +def _new_article(article_seq: count, headline: str, bbox: BoundingBox | None = None) -> ArticleNode: + """Create a new article node with the next deterministic identifier.""" + + return ArticleNode( + article_id=f"article-{next(article_seq)}", + headline=headline, + bbox=bbox, + ) + + def _build_page_dom( content_list: list[dict[str, Any]], *, @@ -49,54 +89,58 @@ def _build_page_dom( text_level = block.get("text_level") role = block.get("role") - if not text and block_type not in {"image", "table"}: + if not text and block_type not in {"image", "table", "chart"}: continue if role == "header": page.headers.append(text) continue + if role == "footer": + page.footers.append(text) + continue + + if role == "page_number": + page.page_numbers.append(text) + continue + if role == "ad" or block_type == "ad": page.ads.append(text) continue - if block_type == "image": + if block_type in {"image", "chart"}: image = ImageNode( - path=block.get("img_path") or block.get("path") or "image", bbox=bbox + path=block.get("img_path") or block.get("path") or block_type, + media_type=block_type, + bbox=bbox, ) - for caption in block.get("image_caption", []): - image.captions.append(CaptionNode(text=str(caption))) + caption_key = f"{block_type}_caption" + footnote_key = f"{block_type}_footnote" + image.captions.extend(_caption_nodes_from_items(block.get(caption_key))) + image.footnotes.extend(_caption_nodes_from_items(block.get(footnote_key))) if current_article is None: - current_article = ArticleNode( - article_id=f"article-{next(article_seq)}", headline="(untitled)" - ) + current_article = _new_article(article_seq, "(untitled)") page.articles.append(current_article) current_article.images.append(image) continue if block_type == "table": if current_article is None: - current_article = ArticleNode( - article_id=f"article-{next(article_seq)}", headline="(table-block)" - ) + current_article = _new_article(article_seq, "(table-block)") page.articles.append(current_article) current_article.body_blocks.append(block.get("table_body", "")) + current_article.captions.extend(_caption_nodes_from_items(block.get("table_caption"))) + current_article.footnotes.extend(_caption_nodes_from_items(block.get("table_footnote"))) continue is_headline = bool(text_level == 1 or role == "section_headings") if is_headline: - current_article = ArticleNode( - article_id=f"article-{next(article_seq)}", - headline=text.replace("\n", " "), - bbox=bbox, - ) + current_article = _new_article(article_seq, text.replace("\n", " "), bbox) page.articles.append(current_article) continue if current_article is None: - current_article = ArticleNode( - article_id=f"article-{next(article_seq)}", headline="(untitled)" - ) + current_article = _new_article(article_seq, "(untitled)") page.articles.append(current_article) current_article.body_blocks.append(text.replace("\n", " ")) diff --git a/src/newsdom_api/errors.py b/src/newsdom_api/errors.py new file mode 100644 index 00000000..90c05878 --- /dev/null +++ b/src/newsdom_api/errors.py @@ -0,0 +1,30 @@ +"""Typed application errors for MinerU execution and output handling.""" + +from __future__ import annotations + + +class MineruRuntimeUnavailableError(RuntimeError): + """MinerU could not be executed successfully.""" + + def __init__( + self, + *, + returncode: int | None = None, + stdout: str | None = None, + stderr: str | None = None, + ) -> None: + """Store internal diagnostics while exposing a sanitized public message.""" + + self.returncode = returncode + self.stdout = stdout + self.stderr = stderr + super().__init__("MinerU runtime unavailable") + + +class MineruIncompleteOutputError(RuntimeError): + """MinerU finished without producing the required artifacts.""" + + def __init__(self) -> None: + """Expose a stable sanitized message for incomplete MinerU artifacts.""" + + super().__init__("MinerU output was incomplete") diff --git a/src/newsdom_api/main.py b/src/newsdom_api/main.py index d985a0cd..054b08b6 100644 --- a/src/newsdom_api/main.py +++ b/src/newsdom_api/main.py @@ -2,10 +2,12 @@ from __future__ import annotations +import asyncio from typing import Annotated -from fastapi import FastAPI, File, UploadFile +from fastapi import FastAPI, File, HTTPException, UploadFile +from .errors import MineruIncompleteOutputError, MineruRuntimeUnavailableError from .schemas import ParseResponse from .service import parse_pdf_bytes @@ -23,4 +25,12 @@ def health() -> dict[str, str]: async def parse(file: Annotated[UploadFile, File(...)]) -> ParseResponse: """Parse an uploaded PDF into the canonical DOM response model.""" - return parse_pdf_bytes(await file.read(), filename=file.filename or "upload.pdf") + try: + pdf_bytes = await file.read() + return await asyncio.to_thread( + parse_pdf_bytes, pdf_bytes, filename=file.filename or "upload.pdf" + ) + except MineruRuntimeUnavailableError as exc: + raise HTTPException(status_code=503, detail=str(exc)) from exc + except MineruIncompleteOutputError as exc: + raise HTTPException(status_code=502, detail=str(exc)) from exc diff --git a/src/newsdom_api/mineru_runner.py b/src/newsdom_api/mineru_runner.py index 24a4b12d..b7bd54a4 100644 --- a/src/newsdom_api/mineru_runner.py +++ b/src/newsdom_api/mineru_runner.py @@ -10,7 +10,7 @@ from pathlib import Path from typing import Any -from fastapi import HTTPException +from .errors import MineruIncompleteOutputError, MineruRuntimeUnavailableError def build_mineru_command( @@ -53,7 +53,7 @@ def _find_output_dir(base_output_dir: Path) -> Path: candidates = list(base_output_dir.glob("*/ocr")) if not candidates: - raise FileNotFoundError(f"No MinerU OCR output found under {base_output_dir}") + raise FileNotFoundError("MinerU OCR output directory was not produced") return candidates[0] @@ -68,29 +68,41 @@ def run_mineru(input_pdf: Path) -> dict[str, Any]: completed = subprocess.run( cmd, check=True, capture_output=True, text=True, timeout=300 ) - except subprocess.TimeoutExpired as e: - raise HTTPException( - status_code=504, detail="OCR processing timed out after 5 minutes" - ) from e - except subprocess.CalledProcessError as e: - raise HTTPException( - status_code=500, - detail=f"MinerU OCR process failed: {e.stderr}", - ) from e - - ocr_dir = _find_output_dir(output_dir) - content_path = ocr_dir / f"{input_pdf.stem}_content_list.json" - if not content_path.exists(): - json_candidates = sorted(ocr_dir.glob("*_content_list.json")) - if not json_candidates: - raise FileNotFoundError("MinerU content list JSON was not produced") - content_path = json_candidates[0] - model_candidates = sorted(ocr_dir.glob("*_model.json")) - if not model_candidates: - raise FileNotFoundError("MinerU model JSON was not produced") + except subprocess.TimeoutExpired as exc: + raise MineruRuntimeUnavailableError( + returncode=-1, + stdout=exc.stdout if exc.stdout else "", + stderr="OCR processing timed out after 5 minutes", + ) from exc + except subprocess.CalledProcessError as exc: + raise MineruRuntimeUnavailableError( + returncode=exc.returncode, + stdout=exc.output, + stderr=exc.stderr, + ) from exc + except FileNotFoundError as exc: + raise MineruRuntimeUnavailableError() from exc + try: + ocr_dir = _find_output_dir(output_dir) + content_path = ocr_dir / f"{input_pdf.stem}_content_list.json" + if not content_path.exists(): + json_candidates = sorted(ocr_dir.glob("*_content_list.json")) + if not json_candidates: + raise FileNotFoundError("MinerU content list JSON was not produced") + content_path = json_candidates[0] + model_candidates = sorted(ocr_dir.glob("*_model.json")) + if not model_candidates: + raise FileNotFoundError("MinerU model JSON was not produced") + except FileNotFoundError as exc: + raise MineruIncompleteOutputError() from exc + try: + content_list = json.loads(content_path.read_text(encoding="utf-8")) + model = json.loads(model_candidates[0].read_text(encoding="utf-8")) + except json.JSONDecodeError as exc: + raise MineruIncompleteOutputError() from exc return { - "content_list": json.loads(content_path.read_text(encoding="utf-8")), - "model": json.loads(model_candidates[0].read_text(encoding="utf-8")), + "content_list": content_list, + "model": model, "stdout": completed.stdout, "stderr": completed.stderr, } diff --git a/src/newsdom_api/schemas.py b/src/newsdom_api/schemas.py index 41144366..3dbd0985 100644 --- a/src/newsdom_api/schemas.py +++ b/src/newsdom_api/schemas.py @@ -27,8 +27,10 @@ class ImageNode(BaseModel): """Image metadata preserved in the canonical page structure.""" path: str + media_type: str = "image" bbox: Optional[BoundingBox] = None captions: List[CaptionNode] = Field(default_factory=list) + footnotes: List[CaptionNode] = Field(default_factory=list) class ArticleNode(BaseModel): @@ -40,6 +42,7 @@ class ArticleNode(BaseModel): body_blocks: List[str] = Field(default_factory=list) images: List[ImageNode] = Field(default_factory=list) captions: List[CaptionNode] = Field(default_factory=list) + footnotes: List[CaptionNode] = Field(default_factory=list) class PageNode(BaseModel): @@ -51,6 +54,8 @@ class PageNode(BaseModel): articles: List[ArticleNode] = Field(default_factory=list) ads: List[str] = Field(default_factory=list) headers: List[str] = Field(default_factory=list) + footers: List[str] = Field(default_factory=list) + page_numbers: List[str] = Field(default_factory=list) class ParseQuality(BaseModel): diff --git a/tests/fixtures/mineru_multi_page_model.json b/tests/fixtures/mineru_multi_page_model.json new file mode 100644 index 00000000..722fddd1 --- /dev/null +++ b/tests/fixtures/mineru_multi_page_model.json @@ -0,0 +1,18 @@ +[ + { + "page_info": { + "page_no": 0, + "width": 1200, + "height": 1800 + }, + "layout_dets": [] + }, + { + "page_info": { + "page_no": 1, + "width": 1280, + "height": 1820 + }, + "layout_dets": [] + } +] diff --git a/tests/fixtures/mineru_multi_page_sample.json b/tests/fixtures/mineru_multi_page_sample.json new file mode 100644 index 00000000..3f4c5288 --- /dev/null +++ b/tests/fixtures/mineru_multi_page_sample.json @@ -0,0 +1,93 @@ +[ + { + "type": "text", + "text": "Synthetic News Page 1", + "role": "header", + "page_idx": 0, + "bbox": [40, 30, 1160, 90] + }, + { + "type": "text", + "text": "Battery Supply Chain Expands", + "text_level": 1, + "page_idx": 0, + "bbox": [80, 140, 620, 220] + }, + { + "type": "text", + "text": "Producers added new refining capacity for cathode materials.", + "page_idx": 0, + "bbox": [80, 240, 620, 520] + }, + { + "type": "image", + "img_path": "images/page-1-photo.png", + "page_idx": 0, + "bbox": [700, 240, 1110, 760], + "image_caption": ["Image caption on page 1"], + "image_footnote": ["Image footnote on page 1"] + }, + { + "type": "text", + "text": "Page 1 footer", + "role": "footer", + "page_idx": 0, + "bbox": [50, 1700, 420, 1760] + }, + { + "type": "text", + "text": "1", + "role": "page_number", + "page_idx": 0, + "bbox": [1080, 1700, 1130, 1760] + }, + { + "type": "text", + "text": "Synthetic News Page 2", + "role": "header", + "page_idx": 1, + "bbox": [40, 30, 1240, 90] + }, + { + "type": "text", + "text": "Market Outlook Highlights", + "role": "section_headings", + "page_idx": 1, + "bbox": [90, 150, 640, 230] + }, + { + "type": "chart", + "img_path": "charts/page-2-growth.png", + "page_idx": 1, + "bbox": [120, 270, 710, 930], + "chart_caption": ["Chart caption on page 2"], + "chart_footnote": ["Chart footnote on page 2"] + }, + { + "type": "table", + "page_idx": 1, + "bbox": [760, 270, 1210, 930], + "table_body": "
North42
", + "table_caption": ["Table caption on page 2"], + "table_footnote": ["Table footnote on page 2"] + }, + { + "type": "text", + "text": "Orphaned text without explicit page index.", + "bbox": [100, 980, 520, 1060] + }, + { + "type": "text", + "text": "Page 2 footer", + "role": "footer", + "page_idx": 1, + "bbox": [60, 1720, 430, 1780] + }, + { + "type": "text", + "text": "2", + "role": "page_number", + "page_idx": 1, + "bbox": [1180, 1720, 1230, 1780] + } +] diff --git a/tests/test_docker_delivery.py b/tests/test_docker_delivery.py index 2e66764c..d1bcdb92 100644 --- a/tests/test_docker_delivery.py +++ b/tests/test_docker_delivery.py @@ -130,6 +130,15 @@ def test_readme_documents_docker_build_and_run(): assert "NVIDIA" in text +def test_readme_describes_default_image_as_shipping_mineru_runtime() -> None: + text = Path("README.md").read_text(encoding="utf-8") + + assert "NEWSDOM_MINERU_BIN=mineru" in text + assert "includes the MinerU runtime" in text + assert "real `/parse` execution" not in text + assert "requires a compatible MinerU runtime to be available inside the container image" not in text + + def test_docker_command_matchers_allow_wrapped_whitespace(): sample = "docker build\n -t newsdom-api .\n\n docker run -p 8000:8000\n newsdom-api" diff --git a/tests/test_dom_builder.py b/tests/test_dom_builder.py index 9b634a44..08f65d6b 100644 --- a/tests/test_dom_builder.py +++ b/tests/test_dom_builder.py @@ -1,7 +1,16 @@ import json from pathlib import Path -from newsdom_api.dom_builder import _bbox_from_values, build_dom +from newsdom_api.dom_builder import ( + _bbox_from_values, + _caption_nodes_from_items, + _coerce_page_number, + build_dom, +) + + +def _load_fixture(name: str): + return json.loads(Path(f"tests/fixtures/{name}").read_text(encoding="utf-8")) def test_build_dom_extracts_articles_from_mineru_sample(): @@ -66,6 +75,82 @@ def test_build_dom_creates_untitled_article_for_plain_text(): assert dom.pages[0].articles[0].headline == "(untitled)" +def test_build_dom_preserves_multi_page_structure_and_page_scoped_metadata(): + sample = _load_fixture("mineru_multi_page_sample.json") + model = _load_fixture("mineru_multi_page_model.json") + dom = build_dom(sample, document_id="doc-multi-page", model=model) + + assert len(dom.pages) == 2 + + first_page = dom.pages[0] + second_page = dom.pages[1] + + assert [page.page_number for page in dom.pages] == [1, 2] + assert (first_page.width, first_page.height) == (1200.0, 1800.0) + assert (second_page.width, second_page.height) == (1280.0, 1820.0) + assert first_page.headers == ["Synthetic News Page 1"] + assert second_page.headers == ["Synthetic News Page 2"] + assert first_page.footers == ["Page 1 footer"] + assert second_page.footers == ["Page 2 footer"] + assert first_page.page_numbers == ["1"] + assert second_page.page_numbers == ["2"] + + first_article = first_page.articles[0] + second_article = second_page.articles[0] + + assert "Page 1 footer" not in first_article.body_blocks + assert "Page 2 footer" not in second_article.body_blocks + assert "1" not in first_article.body_blocks + assert "2" not in second_article.body_blocks + + assert first_article.images[0].path == "images/page-1-photo.png" + assert first_article.images[0].media_type == "image" + assert [caption.text for caption in first_article.images[0].captions] == [ + "Image caption on page 1" + ] + assert [caption.text for caption in first_article.images[0].footnotes] == [ + "Image footnote on page 1" + ] + + assert second_article.images[0].path == "charts/page-2-growth.png" + assert second_article.images[0].media_type == "chart" + assert [caption.text for caption in second_article.images[0].captions] == [ + "Chart caption on page 2" + ] + assert [caption.text for caption in second_article.images[0].footnotes] == [ + "Chart footnote on page 2" + ] + assert second_article.captions[0].text == "Table caption on page 2" + assert second_article.footnotes[0].text == "Table footnote on page 2" + assert any("page_idx" in warning for warning in dom.quality.warnings) + + +def test_coerce_page_number_returns_none_for_type_and_value_errors(): + assert _coerce_page_number(object()) is None + assert _coerce_page_number("not-a-page-number") is None + assert _coerce_page_number(None) is None + assert _coerce_page_number(True) is None + assert _coerce_page_number(False) is None + + +def test_caption_nodes_from_items_uses_contents_and_bbox_variants_and_skips_empty_text(): + nodes = _caption_nodes_from_items( + [ + {"contents": " Caption from contents ", "bbox": [1, 2, 3, 4]}, + {"text": " ", "bbox": [9, 9, 9, 9]}, + {"contents": "Caption from box", "box": [5, 6, 7, 8]}, + ] + ) + + assert [node.text for node in nodes] == ["Caption from contents", "Caption from box"] + assert nodes[0].bbox is not None + assert nodes[0].bbox.x0 == 1.0 + assert nodes[0].bbox.y1 == 4.0 + assert nodes[1].bbox is not None + assert nodes[1].bbox.x0 == 5.0 + assert nodes[1].bbox.y1 == 8.0 + + def test_build_dom_preserves_multi_page_structure_from_page_idx_and_model(): dom = build_dom( [ diff --git a/tests/test_engineering_canonical_docs.py b/tests/test_engineering_canonical_docs.py index 210aaaa9..bbc2b2ba 100644 --- a/tests/test_engineering_canonical_docs.py +++ b/tests/test_engineering_canonical_docs.py @@ -137,3 +137,15 @@ def test_deploy_runbook_matches_release_trigger_and_assets() -> None: assert "release pr lands on `main`" not in runbook_text.lower() for expected in ("SHA256SUMS.txt", "release-manifest.json", "*.intoto.jsonl"): assert expected in runbook_text + + +def test_deploy_runbook_describes_current_runtime_and_probe_contract() -> None: + text = Path("docs/operations/deploy-runbook.md").read_text(encoding="utf-8") + + for expected in ( + "default image already includes the MinerU runtime", + "`NEWSDOM_MINERU_BIN=mineru`", + "`/health` proves the API process is serving but does not validate a full `/parse` round-trip", + "no in-tree Kubernetes manifests", + ): + assert expected in text diff --git a/tests/test_manual_docs.py b/tests/test_manual_docs.py index 69c25c33..5077c77a 100644 --- a/tests/test_manual_docs.py +++ b/tests/test_manual_docs.py @@ -20,6 +20,27 @@ def test_api_reference_examples_are_consistent_and_valid_json(): assert payload["pages"][0]["articles"][0]["images"][0]["bbox"]["x0"] == 120.0 +def test_api_reference_uses_current_runtime_commands_and_temp_file_contract() -> None: + text = Path("manual/api-reference.md").read_text(encoding="utf-8") + + assert "uv run uvicorn --app-dir src newsdom_api.main:app --reload" in text + assert "임시 디렉토리" in text + assert "응답 후 정리됩니다" in text + assert "uvicorn newsdom_api.main:app --reload" not in text + + +def test_api_reference_documents_sanitized_parse_failure_semantics() -> None: + text = Path("manual/api-reference.md").read_text(encoding="utf-8") + + for expected in [ + "503 Service Unavailable", + "MinerU runtime unavailable", + "502 Bad Gateway", + "MinerU output was incomplete", + ]: + assert expected in text + + def test_development_doc_uses_tree_wording(): text = Path("manual/development.md").read_text(encoding="utf-8") assert "트리 구조의 DOM" in text diff --git a/tests/test_mineru_runner.py b/tests/test_mineru_runner.py index 1934b484..c93205e7 100644 --- a/tests/test_mineru_runner.py +++ b/tests/test_mineru_runner.py @@ -3,8 +3,8 @@ from unittest.mock import patch import pytest -from fastapi import HTTPException +from newsdom_api.errors import MineruRuntimeUnavailableError from newsdom_api.mineru_runner import build_mineru_command, run_mineru @@ -18,7 +18,7 @@ def test_build_mineru_command_uses_pipeline_backend(tmp_path: Path): def test_run_mineru_handles_timeout(tmp_path: Path): """ Given a PDF that causes the mineru subprocess to time out, - run_mineru should raise an HTTPException to prevent hanging. + run_mineru should raise an MineruRuntimeUnavailableError to prevent hanging. """ input_pdf = tmp_path / "dummy.pdf" input_pdf.write_text("dummy content") @@ -27,17 +27,17 @@ def test_run_mineru_handles_timeout(tmp_path: Path): patch("shutil.which", return_value="mineru"): mock_run.side_effect = subprocess.TimeoutExpired(cmd="mineru", timeout=0.1) - with pytest.raises(HTTPException) as exc_info: + with pytest.raises(MineruRuntimeUnavailableError) as exc_info: run_mineru(input_pdf) - assert exc_info.value.status_code == 504 - assert "OCR processing timed out" in str(exc_info.value.detail) + assert exc_info.value.returncode == -1 + assert "OCR processing timed out" in str(exc_info.value.stderr) def test_run_mineru_handles_called_process_error(tmp_path: Path): """ Given a PDF that causes a non-zero exit code, - run_mineru should raise an HTTPException with the stderr. + run_mineru should raise an MineruRuntimeUnavailableError with the stderr. """ input_pdf = tmp_path / "dummy.pdf" input_pdf.write_text("dummy content") @@ -48,8 +48,8 @@ def test_run_mineru_handles_called_process_error(tmp_path: Path): returncode=1, cmd="mineru", stderr="Something went wrong" ) - with pytest.raises(HTTPException) as exc_info: + with pytest.raises(MineruRuntimeUnavailableError) as exc_info: run_mineru(input_pdf) - assert exc_info.value.status_code == 500 - assert "Something went wrong" in str(exc_info.value.detail) + assert exc_info.value.returncode == 1 + assert "Something went wrong" in str(exc_info.value.stderr) diff --git a/tests/test_mineru_runner_paths.py b/tests/test_mineru_runner_paths.py index b66bd6b1..00a36a60 100644 --- a/tests/test_mineru_runner_paths.py +++ b/tests/test_mineru_runner_paths.py @@ -1,5 +1,6 @@ import json from pathlib import Path +import subprocess import pytest @@ -18,6 +19,19 @@ def __exit__(self, exc_type, exc, tb): return False +def _assert_no_private_path_material(value: str) -> None: + forbidden_fragments = ( + "/Users/", + "/private/var/folders/", + "/tmp/", + "\\Users\\", + "\\Temp\\", + "newsdom-mineru-", + ) + for fragment in forbidden_fragments: + assert fragment not in value + + def test_resolve_mineru_bin_prefers_env(monkeypatch): monkeypatch.setenv("NEWSDOM_MINERU_BIN", "/opt/mineru") assert mineru_runner._resolve_mineru_bin() == "/opt/mineru" @@ -115,14 +129,102 @@ class Result: assert result["content_list"][0]["text"] == "exact" -def test_run_mineru_raises_when_content_json_missing(monkeypatch, tmp_path: Path): +def test_run_mineru_wraps_called_process_error(monkeypatch, tmp_path: Path): tempdir = tmp_path / "temp" - ocr_dir = tempdir / "sample" / "ocr" - ocr_dir.mkdir(parents=True) - (ocr_dir / "alt_model.json").write_text( - json.dumps([{"layout_dets": []}]), encoding="utf-8" + tempdir.mkdir() + + monkeypatch.setenv("NEWSDOM_MINERU_BIN", "/opt/mineru") + monkeypatch.setattr( + mineru_runner.tempfile, + "TemporaryDirectory", + lambda prefix: _FakeTempDir(tempdir), ) + def fake_run(cmd, check, capture_output, text, timeout=None): + assert check is True + assert capture_output is True + assert text is True + raise subprocess.CalledProcessError( + returncode=23, + cmd=cmd, + output="runtime output from /private/var/folders/secret", + stderr="runtime stderr from /Users/private-user/tmp/mineru.log", + ) + + monkeypatch.setattr(mineru_runner.subprocess, "run", fake_run) + + with pytest.raises(Exception) as exc_info: + mineru_runner.run_mineru(Path("sample.pdf")) + + assert exc_info.type.__name__ == "MineruRuntimeUnavailableError" + assert exc_info.value.returncode == 23 + assert exc_info.value.stdout == "runtime output from /private/var/folders/secret" + assert exc_info.value.stderr == "runtime stderr from /Users/private-user/tmp/mineru.log" + _assert_no_private_path_material(str(exc_info.value)) + + +def test_run_mineru_wraps_missing_executable_failure(monkeypatch, tmp_path: Path): + tempdir = tmp_path / "temp" + tempdir.mkdir() + + monkeypatch.setenv("NEWSDOM_MINERU_BIN", "/opt/private/mineru") + monkeypatch.setattr( + mineru_runner.tempfile, + "TemporaryDirectory", + lambda prefix: _FakeTempDir(tempdir), + ) + + def fake_run(cmd, check, capture_output, text, timeout=None): + assert check is True + assert capture_output is True + assert text is True + raise FileNotFoundError("/opt/private/mineru not found in /Users/private-user/bin") + + monkeypatch.setattr(mineru_runner.subprocess, "run", fake_run) + + with pytest.raises(Exception) as exc_info: + mineru_runner.run_mineru(Path("sample.pdf")) + + assert exc_info.type.__name__ == "MineruRuntimeUnavailableError" + assert getattr(exc_info.value, "returncode", None) is None + assert getattr(exc_info.value, "stdout", None) in (None, "") + assert getattr(exc_info.value, "stderr", None) in (None, "") + _assert_no_private_path_material(str(exc_info.value)) + + +@pytest.mark.parametrize( + ("fixture_name", "create_outputs"), + [ + ("missing ocr dir", lambda output_dir: None), + ( + "missing content list", + lambda output_dir: (output_dir / "sample" / "ocr").mkdir(parents=True), + ), + ( + "missing model", + lambda output_dir: ( + (output_dir / "sample" / "ocr").mkdir(parents=True), + (output_dir / "sample" / "ocr" / "alt_content_list.json").write_text( + json.dumps([{"type": "text", "text": "ok"}]), + encoding="utf-8", + ), + ), + ), + ], + ids=["missing-ocr-dir", "missing-content-list", "missing-model"], +) +def test_run_mineru_raises_typed_incomplete_output_error( + monkeypatch, tmp_path: Path, fixture_name: str, create_outputs +): + tempdir = tmp_path / "temp" + create_outputs(tempdir) + if fixture_name == "missing content list": + ocr_dir = tempdir / "sample" / "ocr" + (ocr_dir / "alt_model.json").write_text( + json.dumps([{"layout_dets": []}]), encoding="utf-8" + ) + + monkeypatch.setenv("NEWSDOM_MINERU_BIN", "/opt/mineru") monkeypatch.setattr(mineru_runner.shutil, "which", lambda name: "/usr/bin/mineru") monkeypatch.setattr( mineru_runner.tempfile, @@ -144,17 +246,37 @@ class Result: monkeypatch.setattr(mineru_runner.subprocess, "run", fake_run) - with pytest.raises(FileNotFoundError): + with pytest.raises(Exception) as exc_info: mineru_runner.run_mineru(Path("sample.pdf")) - -def test_run_mineru_raises_when_model_json_missing(monkeypatch, tmp_path: Path): + assert exc_info.type.__name__ == "MineruIncompleteOutputError" + _assert_no_private_path_material(str(exc_info.value)) + + +@pytest.mark.parametrize( + ("artifact_name", "file_name"), + [ + ("content_list", "sample_content_list.json"), + ("model", "sample_model.json"), + ], + ids=["malformed-content-list", "malformed-model"], +) +def test_run_mineru_raises_typed_incomplete_output_error_for_malformed_json( + monkeypatch, tmp_path: Path, artifact_name: str, file_name: str +): tempdir = tmp_path / "temp" ocr_dir = tempdir / "sample" / "ocr" ocr_dir.mkdir(parents=True) - (ocr_dir / "alt_content_list.json").write_text( - json.dumps([{"type": "text", "text": "ok"}]), encoding="utf-8" - ) + + content_payload = "{not valid json" + model_payload = "{not valid json" + if artifact_name == "content_list": + model_payload = json.dumps([{"layout_dets": []}]) + else: + content_payload = json.dumps([{"type": "text", "text": "ok"}]) + + (ocr_dir / "sample_content_list.json").write_text(content_payload, encoding="utf-8") + (ocr_dir / "sample_model.json").write_text(model_payload, encoding="utf-8") monkeypatch.setattr(mineru_runner.shutil, "which", lambda name: "/usr/bin/mineru") monkeypatch.setattr( @@ -177,5 +299,8 @@ class Result: monkeypatch.setattr(mineru_runner.subprocess, "run", fake_run) - with pytest.raises(FileNotFoundError): + with pytest.raises(Exception) as exc_info: mineru_runner.run_mineru(Path("sample.pdf")) + + assert exc_info.type.__name__ == "MineruIncompleteOutputError" + _assert_no_private_path_material(str(exc_info.value)) diff --git a/tests/test_parse_endpoint.py b/tests/test_parse_endpoint.py index 45404915..752123b1 100644 --- a/tests/test_parse_endpoint.py +++ b/tests/test_parse_endpoint.py @@ -1,9 +1,103 @@ from fastapi.testclient import TestClient +import subprocess +from pathlib import Path from newsdom_api.main import app +from newsdom_api import mineru_runner + + +class _FakeTempDir: + def __init__(self, path: Path): + self.path = path + + def __enter__(self): + self.path.mkdir(parents=True, exist_ok=True) + return str(self.path) + + def __exit__(self, exc_type, exc, tb): + return False + + +def _assert_no_private_path_material(value: str) -> None: + forbidden_fragments = ( + "/Users/", + "/private/var/folders/", + "/tmp/", + "\\Users\\", + "\\Temp\\", + "newsdom-upload-", + "newsdom-mineru-", + ) + for fragment in forbidden_fragments: + assert fragment not in value def test_parse_endpoint_requires_pdf_file(): client = TestClient(app) response = client.post("/parse") assert response.status_code == 422 + + +def test_parse_endpoint_returns_503_for_mineru_runtime_failure(monkeypatch): + def fake_run(cmd, check, capture_output, text, timeout=None): + assert check is True + assert capture_output is True + assert text is True + raise subprocess.CalledProcessError( + returncode=17, + cmd=cmd, + output="stdout from /private/var/folders/runtime-output", + stderr="stderr from /Users/private-user/tmp/mineru.stderr", + ) + + monkeypatch.setattr(mineru_runner.subprocess, "run", fake_run) + + client = TestClient(app, raise_server_exceptions=False) + response = client.post( + "/parse", + files={"file": ("fixture.pdf", b"%PDF-1.4\n%synthetic\n", "application/pdf")}, + ) + + assert response.status_code == 503 + assert response.json()["detail"] == "MinerU runtime unavailable" + _assert_no_private_path_material(response.json()["detail"]) + + +def test_parse_endpoint_returns_502_for_incomplete_mineru_output( + monkeypatch, tmp_path: Path +): + tempdir = tmp_path / "mineru-output" + ocr_dir = tempdir / "fixture" / "ocr" + ocr_dir.mkdir(parents=True) + (ocr_dir / "alt_content_list.json").write_text( + '[{"type": "text", "text": "ok"}]', encoding="utf-8" + ) + + monkeypatch.setattr( + mineru_runner.tempfile, + "TemporaryDirectory", + lambda prefix: _FakeTempDir(tempdir), + ) + + def fake_run(cmd, check, capture_output, text, timeout=None): + assert check is True + assert capture_output is True + assert text is True + + class Result: + stdout = "stdout from /private/var/folders/runtime-output" + stderr = "stderr from /Users/private-user/tmp/mineru.stderr" + + return Result() + + monkeypatch.setattr(mineru_runner.subprocess, "run", fake_run) + + client = TestClient(app, raise_server_exceptions=False) + response = client.post( + "/parse", + files={"file": ("fixture.pdf", b"%PDF-1.4\n%synthetic\n", "application/pdf")}, + ) + + assert response.status_code == 502 + assert response.json()["detail"] == "MinerU output was incomplete" + _assert_no_private_path_material(response.json()["detail"]) diff --git a/tests/test_service.py b/tests/test_service.py index 45e707fe..90cd3f98 100644 --- a/tests/test_service.py +++ b/tests/test_service.py @@ -1,3 +1,4 @@ +import json from pathlib import Path from newsdom_api.schemas import ParseResponse @@ -6,7 +7,9 @@ def test_parse_pdf_bytes_writes_temp_file_and_builds_dom(monkeypatch): observed = {} - mineru_model = [{"page_info": {"page_no": 0, "width": 100.0, "height": 200.0}}] + model = json.loads( + Path("tests/fixtures/mineru_multi_page_model.json").read_text(encoding="utf-8") + ) def fake_run_mineru(path: Path): observed["path_name"] = path.name @@ -20,7 +23,7 @@ def fake_run_mineru(path: Path): "bbox": [0, 0, 1, 1], } ], - "model": mineru_model, + "model": model, } def fake_build_dom(content_list, document_id: str, model=None) -> ParseResponse: @@ -36,7 +39,15 @@ def fake_build_dom(content_list, document_id: str, model=None) -> ParseResponse: assert observed["path_name"] == "fixture.pdf" assert observed["bytes"] == b"pdf-bytes" assert observed["document_id"] == "fixture" - assert observed["model"] == mineru_model + assert observed["content_list"] == [ + { + "type": "text", + "text": "headline", + "text_level": 1, + "bbox": [0, 0, 1, 1], + } + ] + assert observed["model"] == model assert result.document_id == "fixture"