generated from john/python-template
Jim LancasterandCopilot App
de18c2e9da
Quality Gate / gate (push) Failing after 48s
Phase 1 of docs/reviews/2026-08-23-code-review.md. HIGH-01: process_queued_job committed page evidence and the terminal job status in separate transactions, so a crash between them left a transcript persisted against a job stuck in PROCESSING that the worker never reclaims. The final page's write is now deferred into _finalize_batch_outcome so it shares the terminal transaction. Intermediate pages remain individually durable, and the terminal commit is shielded against cancellation the same way per-page writes already were. HIGH-04: added tests/integration/test_pipeline_atomicity.py covering both Transaction B and Transaction C. Confirmed failing against the previous implementation before the fix. HIGH-03: classify_unexpected_error interpolated the raw exception into AppError.message, which the UI renders and the API serializes, leaking the database path from OperationalError. message is now generic. Because message also feeds format_error_detail, which writes evidence records, the root cause is preserved on a new internal-only AppError.detail field rather than discarded. HIGH-02: replaced 8 hand-rolled ui.notify error calls in home_page and people_page with error_presenter.show_error, restoring the correlation error_id, canonical category, and suggestion. Added an AST guard to test_ui_boundaries.py so pages cannot hand-roll error notifications again. Docs updated per documentation-sync: the message/detail split in docs/error_handling.md and the multi-page atomicity rule in services.instructions.md. Verification: ruff clean, 381 tests passing, ty unchanged at 10 known SQLAlchemy descriptor false positives. Co-authored-by: Copilot App <[email protected]>
134 lines
5.9 KiB
Markdown
134 lines
5.9 KiB
Markdown
# Error Handling Policy (Current Baseline: V5.1)
|
|
|
|
This policy defines the active V5.1 error taxonomy, translation boundaries, and retry semantics.
|
|
|
|
## Error Categories
|
|
|
|
| Category | Meaning | Typical Origin | User Treatment |
|
|
| :--- | :--- | :--- | :--- |
|
|
| `validation` | Input payload/selection is invalid | UI form parsing, service validators | Inline correction guidance |
|
|
| `not_found` | Target record is missing | ID lookup in service layer | Non-blocking warning or redirect |
|
|
| `conflict` | State prevents requested action | lifecycle transitions, duplicate semantic keys | Explain required precondition |
|
|
| `external` | Provider/network dependency failure | OpenRouter/provider adapter | Retry path and evidence retained |
|
|
| `timeout` | Provider call exceeded configured bound | worker/provider client timeout | Retry path and bounded messaging |
|
|
| `internal` | Unexpected local failure | unhandled service/runtime faults | Safe generic message + diagnostics capture |
|
|
|
|
## Runtime Taxonomy and Canonical Mapping
|
|
|
|
Runtime code uses a richer internal taxonomy for diagnostics and persisted evidence, then maps that
|
|
taxonomy to the six canonical categories at the API/UI envelope boundary.
|
|
|
|
### Internal runtime categories
|
|
|
|
- `validation_error`
|
|
- `user_input_error`
|
|
- `not_found_error`
|
|
- `conflict_error`
|
|
- `external_provider_error`
|
|
- `external_timeout_error`
|
|
- `processing_error`
|
|
- `infrastructure_transient_error`
|
|
- `infrastructure_persistent_error`
|
|
- `internal_unexpected_error`
|
|
|
|
### Internal -> Canonical mapping
|
|
|
|
| Internal category | Canonical envelope category |
|
|
| :--- | :--- |
|
|
| `validation_error` | `validation` |
|
|
| `user_input_error` | `validation` |
|
|
| `not_found_error` | `not_found` |
|
|
| `conflict_error` | `conflict` |
|
|
| `external_provider_error` | `external` |
|
|
| `external_timeout_error` | `timeout` |
|
|
| `infrastructure_transient_error` | `timeout` |
|
|
| `processing_error` | `internal` |
|
|
| `infrastructure_persistent_error` | `internal` |
|
|
| `internal_unexpected_error` | `internal` |
|
|
|
|
`ExecutionAttempt.error_category` stores the internal category value so diagnostics remain specific.
|
|
|
|
## Translation Boundaries
|
|
|
|
- **Provider layer:** raise provider-scoped exceptions with provider context; do not emit UI text.
|
|
- **Service layer:** map raw exceptions into internal categories and preserve causal chain.
|
|
- **UI/API layer:** convert internal categories to canonical categories using the centralized mapping.
|
|
|
|
## Decision Context
|
|
|
|
### Why taxonomy is category-based (not exception-class-based)
|
|
|
|
- Categories encode operator-facing recovery semantics (fix input, retry later, investigate internal failure) independent of low-level exception type.
|
|
- This keeps retry and messaging behavior consistent even when provider/client libraries change.
|
|
|
|
### Why page-level failure is isolated
|
|
|
|
- Multi-page archival documents often contain a mix of readable and degraded pages.
|
|
- Isolating failures to page scope preserves successful results and avoids all-or-nothing loss when one page fails.
|
|
- Aggregate job status then communicates overall outcome (`transcribed`, `partial_success`, `failed`) without hiding page detail.
|
|
|
|
### Why retries append evidence instead of mutating rows
|
|
|
|
- Retry operations are new observations, not corrections of history.
|
|
- Appending attempts preserves forensic traceability, timing history, and provider variability analysis.
|
|
- Projection updates remain explicit user/workflow decisions, separate from immutable evidence.
|
|
|
|
## Job and Page Failure Semantics
|
|
|
|
### Page-Level (`JobSource`)
|
|
|
|
- `pending` -> `transcribed` when attempt succeeds.
|
|
- `pending` -> `failed` when attempt fails terminally.
|
|
- `pending` -> `cancelled` on job cancellation before processing.
|
|
|
|
### Job-Level (`Job`)
|
|
|
|
- `transcribed` when all pages transcribe successfully.
|
|
- `partial_success` when mixed success/failure outcomes exist.
|
|
- `failed` when no page transcribes successfully.
|
|
|
|
## Retry and Retranscription Rules
|
|
|
|
1. Failed/cancelled pages may be re-queued through retranscription workflows.
|
|
2. Retry attempts must append new `ExecutionAttempt` rows; prior evidence remains immutable.
|
|
3. Selecting a better candidate must update projection pointers, not mutate historical attempt rows.
|
|
|
|
## Logging and Diagnostics Rules
|
|
|
|
1. Persist sufficient attempt error metadata (`error_category`, `error_message`, transport evidence) for post-hoc analysis.
|
|
2. Avoid leaking stack traces or local paths into user-facing message envelopes.
|
|
3. Preserve causal exception chains for internal diagnostics.
|
|
|
|
### Message vs detail split
|
|
|
|
Rules 1 and 2 pull in opposite directions: evidence records need the root cause, and
|
|
user-facing envelopes must not carry it. `AppError` therefore separates the two audiences:
|
|
|
|
| Field | Audience | Carries root cause | Surfaces |
|
|
| --- | --- | --- | --- |
|
|
| `message` | User-facing and API-facing | No | `show_error`, `build_error_envelope` |
|
|
| `detail` | Internal only | Yes | `format_error_detail` (evidence), logs |
|
|
|
|
`classify_unexpected_error` builds a generic `message` and puts the exception type and
|
|
text on `detail`. Anything rendered to a user or serialized into an API envelope must
|
|
read `message`; anything persisted as provenance or logged may read `detail`.
|
|
Enforced by `tests/test_errors.py::test_unexpected_error_does_not_leak_filesystem_paths`.
|
|
|
|
## Operator Recovery Guidance
|
|
|
|
- **validation/conflict:** correct input or state and retry manually.
|
|
- **external/timeout:** allow bounded retries and keep prior attempt evidence visible.
|
|
- **internal:** stop automatic retries, surface a safe message, and inspect diagnostics with correlation context.
|
|
|
|
## UI Messaging Contract
|
|
|
|
- User-visible errors must be actionable, bounded, and category-consistent.
|
|
- Multi-page jobs must show partial outcomes instead of collapsing into a single opaque failure.
|
|
- Recovery actions (`retry`, `retranscribe`, `edit input`) must be offered where available.
|
|
|
|
## Cross-Reference
|
|
|
|
- [Error Handling invariant](./invariant/error_handling.md)
|
|
- [System Requirements](requirements.md)
|
|
- [Data Model](schema.md)
|