Three small drops that pay back later:
- Note that main.metrics is import-time bound and tests must
monkeypatch `main.metrics`, not the registry.
- Flag the /metrics endpoint as unauthenticated and cross-link the
Caddy-block snippet in docs/OBSERVABILITY.md.
- Pin prometheus-client to the exact 0.25.0 patch version already
resolved by prometheus-fastapi-instrumentator 7.0.0, so an upstream
bump cannot silently slip in.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Each metrics test was repeating the same five-line block — patch
kraken_engine.load_models, patch load_spell_checker, instantiate the
AsyncClient, force _models_ready True, restore it. Lift the lot into a
single async context manager so each test body shrinks to its real
arrange / act / assert intent.
Tests that drive the lifespan directly (models_ready gauge) or stub
asyncio.to_thread for /train (which already patches _models_ready) stay
unchanged.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The /train, /train-sender, and /segtrain endpoints each duplicated the
same eight-line try/except + counter + gauge block around the
asyncio.to_thread call. Lift it into _record_training(runner, kind),
which accepts a sync- or async-returning callable for flexibility.
Each endpoint now ends with a single return line. Behaviour preserved —
status codes, error propagation, and metric labels stay identical.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The two block-iteration loops (/ocr and /ocr/stream's standard generator)
both ran the same word-total and illegible-word increments. Lift them
into a single helper so each call site becomes one line and the counter
intent reads cleanly. Pure refactor — no behavior change, tests stay green.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The asyncio.to_thread patch stubbed out the entire _run_training call,
hiding the real error path. Replacing it with a failing CompletedProcess
from subprocess.run exercises the actual ketos-failed branch and keeps
the test's intent — error counter bumps, 500 surfaces — intact.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
If uvicorn's access log format ever changes (args=None, or shorter
than 3 elements), the filter must keep forwarding records rather than
silently dropping them. Two extra LogRecords cover both edge cases.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Two regression tests:
- /train-sender hitting the success path bumps the recognition counter
(previously only /train and /segtrain were covered).
- A successful run whose result.accuracy is None must not call set() on
ocr_model_accuracy — the gauge stays at its default 0.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Previously the guided generator's page_started timer wrapped the entire
region loop including the synchronous correct_text() call, inflating
ocr_processing_seconds with spell-check latency. Sum the per-region
engine.extract_region_text durations instead so the histogram matches
the unguided stream's "engine only" semantic.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Locks in the post-download placement of the counter increment so a
regression that moves it back above _download_and_convert_pdf would fail.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds a logging.Filter on uvicorn.access that drops records whose request
path is /metrics or /health. Each is hit on a tight schedule (Prometheus
scrape interval and Docker healthcheck), so unfiltered they dominate the
access log without carrying any information about real traffic.
Refs #652 (Nora's recommendation)
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Mirrors the existing _models_ready bool so Prometheus has a time-series
liveness/readiness signal for future alerting rules (e.g.
ocr_models_ready < 1 for 2m).
Refs #652 (AC7)
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Hits /train then /segtrain through the same test, each with a distinct
mocked accuracy, and asserts the labelled gauges reflect the two values.
Locks down the kind-label separation between recognition and segmentation
accuracy (decision #2).
Refs #652 (AC6)
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Wraps the await asyncio.to_thread(_run_*) calls in /train, /train-sender,
and /segtrain with try/except. Recognition training (/train, /train-sender)
shares kind="recognition"; /segtrain uses kind="segmentation". The
ocr_model_accuracy gauge is set per kind on success.
Refs #652 (AC6, decision #2)
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Wraps every asyncio.to_thread(engine.extract_*) call with time.monotonic()
deltas in /ocr (per document) and in both /ocr/stream generators (per page).
Streaming buckets are the useful operational signal; the non-streaming
observation is a bonus.
Refs #652 (AC5)
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Walks block["words"] before apply_confidence_markers strips the list, then
increments ocr_words_total by len(words) and ocr_illegible_words_total by
the count below threshold. Same pattern in both /ocr and /ocr/stream so the
ratio illegible/words is a faithful quality signal across endpoints.
Refs #652 (AC4)
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Bumps the counter in both /ocr/stream except blocks (standard and guided
generators) so the existing skipped_pages local variable now also flows
into Prometheus.
Refs #652 (AC3b)
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Bumps the counter inside both the standard and guided /ocr/stream
generators after a page yields its blocks, before the per-page json line is
emitted. Also moves the ocr_jobs_total increment for /ocr/stream right after
engine selection so the counter still fires when a page later errors out.
Refs #652 (AC3a)
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Locks down AC2 for the non-Kurrent path. The same code branch in /ocr that
sets engine_name from script_type now has explicit coverage for both
HANDWRITING_KURRENT → kraken and TYPEWRITER → surya.
Refs #652 (AC2)
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Pick engine="kraken" for HANDWRITING_KURRENT, engine="surya" otherwise,
then increment after the blocks have been extracted.
Refs #652 (AC2)
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Encapsulates every custom OCR metric in an OcrMetrics frozen dataclass and
exposes a `build_metrics(registry)` factory. Production main.py binds against
the default REGISTRY; tests construct a fresh CollectorRegistry per case and
monkeypatch main.metrics, so counter values stay isolated between tests
(decision #3 on issue #652, Option A).
Refs #652
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Locks down AC1: prometheus-fastapi-instrumentator must keep auto-exposing
http_requests_total and http_request_duration_seconds for application
traffic, not just register the /metrics endpoint.
Refs #652
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Mount the instrumentator immediately after FastAPI app creation, excluding
/health and /metrics from request metrics to keep http_requests_total focused
on real application traffic.
Refs #652
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- entrypoint.sh: replace "cross-job ground-truth leakage" with plain
"Remove stale partial downloads left by a previous docker-kill"
- test_tmpdir_is_inside_persistent_cache_volume: add docker exec command
so future developers know how to run this deployment-contract test
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
test_entrypoint_removes_day_old_orphans and test_entrypoint_preserves_fresh_files
verify the find -mtime +1 -delete logic using os.utime() to fabricate old mtimes
without mocking system time. Also extracts _run_entrypoint helper to remove
subprocess setup duplication.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
A silent non-zero exit would previously cause the test to pass incorrectly
because only directory creation was checked. Exit code is now the first
assertion, catching regressions before the filesystem check runs.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
_validate_zip_entry has no ML-stack dependency; importing it via main.py
pulled in surya/torch and caused the test to be skipped in CI. Moving it
to utils.py (fastapi only) and adding fastapi to the CI lightweight install
lets test_zipslip_still_anchors_under_custom_tmpdir run on every push.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- ocr-service/README.md: add HF_HOME, XDG_CACHE_HOME, TORCH_HOME, TMPDIR rows
to the environment variables table
- ocr-service/CLAUDE.md: LLM reminder — TMPDIR must stay on the cache volume
- docs/adr/021-tmpdir-persistent-volume-staging.md: records the decision,
trade-offs, and rejected alternatives (Approach B / C) for issue #614
- ci.yml: add test_tmpdir.py to the OCR CI run (stdlib-only tests, no ML stack)
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
On a fresh ocr_cache volume /app/cache/.tmp does not exist yet. The mkdir
ensures the first Surya model download can proceed without ENOSPC on the
512 MB /tmp tmpfs. The find cleanup removes fragments left by docker-kill
mid-download, preventing cross-job ground-truth leakage.
Fixes#614. See ADR-021.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Without this, running the image outside compose loses the TMPDIR redirect
and Surya model downloads fall back to the 512 MB /tmp tmpfs (ENOSPC).
See issue #614, ADR-021.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
os.environ.get(key, default) returns "" when the key exists but is blank —
the default is only used when the key is absent. The or-fallback treats both
absence and blank values as "use the default".
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Prevents PyTorch/Matplotlib/Ketos from writing to /home/ocr which is
on the read-only container filesystem — fixes Nora's blocker. Also
restores the explanatory comment on the ocr_cache volume mount.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds a canary log line if os.getuid() == 0. Produces an observable
signal in container logs if the USER directive is ever removed from
the Dockerfile, without requiring an external audit tool.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
With --no-create-home, os.path.expanduser("~") resolves to "/" causing
kraken get to write to /.local/share/htrmopo. Replace with
os.environ.get("HTRMOPO_DIR", "/app/models/.htrmopo") so the path is
explicit and override-friendly without a home directory.
Adds two tests verifying env-var resolution and ~-free default.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
CIS Docker §4.1: run uvicorn as UID 1000 (ocr) instead of root.
Creates /home/ocr and /app/cache with correct ownership so named
volumes inherit ocr:ocr on first Docker mount. Sets HOME and HF_HOME
so ~ expansion and Hugging Face caching resolve under /app, not /root.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- docs/README.md: remove duplicate infrastructure/ entry at end of folder tree
- ocr-service/CLAUDE.md: add **LLM reminder:** prefix to ALLOWED_PDF_HOSTS
SSRF warning (consistent with all other machine-readable instructions)
- backend/CLAUDE.md: restore ResponseStatusException note for simple controller
validation — avoids LLMs reaching for DomainException for trivial checks
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
extract_page_blocks() walked `record.boundary` and `record.baseline`
unconditionally, so a record that arrived without either (malformed
kraken output, or a MagicMock in tests that iterates to nothing)
crashed with "min() arg is an empty sequence".
Coerce both attributes through list(), require at least 3 points for
the polygon path, fall back to the baseline path when the polygon is
missing, and skip the record entirely when neither is usable —
emitting no block is safer than emitting one with garbage coordinates.
The test helper now sets `boundary` and `baseline` explicitly to
mirror real Kraken 7.0 records (and so the happy-path test exercises
the polygon branch). A new regression test covers the skip path.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
main.py unifies the call to both engines and always passes
`sender_model_path` (None for non-Kurrent scripts). Surya's
extract_region_text / extract_page_blocks accepted one fewer positional
arg than Kraken's, so every guided-OCR run on a TYPEWRITER or
HANDWRITING_LATIN document raised "takes 5 positional arguments but 6
were given" and the stream returned 0 blocks / 1 skipped page.
Add an ignored `sender_model_path` kwarg to both Surya functions so the
signatures match Kraken's, and guard the regression with two signature
tests in test_engines.py that compare both engines' parameter lists.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Add 503/403 auth tests for the /train-sender endpoint, matching the pattern
already used for /train and /segtrain. Also surface test_sender_registry.py
in CI (it needs no ML stack) and add pytest-asyncio to the install step.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace exact-string assertions in test_correctable_ocr_error_gets_corrected
and test_sentence_with_multiple_corrections with structural assertions that
verify behavior (correction attempted, marker present, expected stem) without
coupling to a specific pyspellchecker version's frequency weights.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Strict greater-than avoids non-determinism: if multiple candidates share
the minimum frequency value, pyspellchecker's ranking is undefined.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
