generated from john/python-template
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]>
99 lines
4.5 KiB
Python
99 lines
4.5 KiB
Python
"""Tests for shared error taxonomy and helpers."""
|
|
|
|
import pytest
|
|
|
|
from transcription.errors import AppError
|
|
from transcription.errors import ErrorCategory
|
|
from transcription.errors import build_error_envelope
|
|
from transcription.errors import canonical_error_category
|
|
from transcription.errors import classify_unexpected_error
|
|
from transcription.errors import format_error_detail
|
|
from transcription.errors import new_error_id
|
|
|
|
|
|
@pytest.mark.unit
|
|
class TestErrorCategoryContract:
|
|
"""Verify stable category identifiers."""
|
|
|
|
def test_category_values_match_policy_contract(self):
|
|
"""Error category values match docs/error_handling.md identifiers."""
|
|
assert ErrorCategory.VALIDATION.value == "validation_error"
|
|
assert ErrorCategory.USER_INPUT.value == "user_input_error"
|
|
assert ErrorCategory.NOT_FOUND.value == "not_found_error"
|
|
assert ErrorCategory.CONFLICT.value == "conflict_error"
|
|
assert ErrorCategory.EXTERNAL_PROVIDER.value == "external_provider_error"
|
|
assert ErrorCategory.EXTERNAL_TIMEOUT.value == "external_timeout_error"
|
|
assert ErrorCategory.PROCESSING.value == "processing_error"
|
|
assert ErrorCategory.INFRA_TRANSIENT.value == "infrastructure_transient_error"
|
|
assert ErrorCategory.INFRA_PERSISTENT.value == "infrastructure_persistent_error"
|
|
assert ErrorCategory.INTERNAL_UNEXPECTED.value == "internal_unexpected_error"
|
|
|
|
|
|
@pytest.mark.unit
|
|
class TestAppErrorHelpers:
|
|
"""Verify helper behavior for IDs and normalization."""
|
|
|
|
def test_new_error_id_returns_short_identifier(self):
|
|
"""new_error_id returns a short non-empty identifier."""
|
|
value = new_error_id()
|
|
assert isinstance(value, str)
|
|
assert len(value) == 8
|
|
|
|
def test_classify_unexpected_error_returns_internal_unexpected(self):
|
|
"""Unexpected exceptions are normalized to internal_unexpected_error."""
|
|
err = classify_unexpected_error(RuntimeError("boom"), operation="unit.test")
|
|
|
|
assert isinstance(err, AppError)
|
|
assert err.category == ErrorCategory.INTERNAL_UNEXPECTED
|
|
assert "unit.test" in err.message
|
|
# The raw exception text must stay out of the user-facing message: it is rendered
|
|
# by the UI presenter and serialized into API envelopes, and unexpected exceptions
|
|
# routinely embed local filesystem paths.
|
|
assert "boom" not in err.message
|
|
assert err.suggestion
|
|
assert err.error_id
|
|
|
|
def test_unexpected_error_does_not_leak_filesystem_paths(self):
|
|
"""User-facing and API-facing text must not carry local filesystem paths.
|
|
|
|
`.github/instructions/error-handling.instructions.md` forbids leaking local
|
|
filesystem paths in user-facing output. A SQLAlchemy OperationalError embeds the
|
|
database path and an OSError embeds the storage root, so the generic catch-all
|
|
path is where that leak would occur. The cause is retained on `detail`, which is
|
|
internal-only, so evidence records and logs keep full diagnostic value.
|
|
"""
|
|
secret_path = r"C:\Github\transcription\data\transcription.db"
|
|
exc = OSError(f"unable to open database file: {secret_path}")
|
|
|
|
err = classify_unexpected_error(exc, operation="worker.process_job")
|
|
envelope = build_error_envelope(err)
|
|
|
|
assert secret_path not in err.message
|
|
assert secret_path not in envelope.message
|
|
assert secret_path not in err.suggestion
|
|
|
|
# Internal surfaces keep the root cause.
|
|
assert err.detail is not None
|
|
assert secret_path in err.detail
|
|
assert secret_path in format_error_detail(err)
|
|
|
|
def test_envelope_categories_use_canonical_contract_values(self):
|
|
"""API/UI envelope categories are normalized to canonical short identifiers."""
|
|
expected_mapping = {
|
|
ErrorCategory.VALIDATION: "validation",
|
|
ErrorCategory.USER_INPUT: "validation",
|
|
ErrorCategory.NOT_FOUND: "not_found",
|
|
ErrorCategory.CONFLICT: "conflict",
|
|
ErrorCategory.EXTERNAL_PROVIDER: "external",
|
|
ErrorCategory.EXTERNAL_TIMEOUT: "timeout",
|
|
ErrorCategory.INFRA_TRANSIENT: "timeout",
|
|
ErrorCategory.PROCESSING: "internal",
|
|
ErrorCategory.INFRA_PERSISTENT: "internal",
|
|
ErrorCategory.INTERNAL_UNEXPECTED: "internal",
|
|
}
|
|
|
|
for category, expected in expected_mapping.items():
|
|
err = AppError("x", category=category)
|
|
assert canonical_error_category(err) == expected
|
|
assert build_error_envelope(err).category == expected
|