M54: harden operations and demo resilience
This commit is contained in:
@@ -2,7 +2,7 @@ from __future__ import annotations
|
||||
|
||||
import uuid
|
||||
from dataclasses import dataclass, field
|
||||
from datetime import UTC, datetime, timedelta
|
||||
from datetime import UTC, datetime
|
||||
|
||||
from sqlalchemy import func, select, update
|
||||
from sqlalchemy.orm import Session
|
||||
@@ -11,10 +11,22 @@ from app.core.errors import AppError
|
||||
from app.models.booking import Booking
|
||||
from app.models.customer import Customer
|
||||
from app.models.data_quality import DataQualityIssue
|
||||
from app.models.inspection import Inspection
|
||||
from app.models.maintenance import MaintenanceRecord
|
||||
from app.models.vehicle import Vehicle
|
||||
from app.schemas import CurrentUser, ResolveOdometerRegressionRequest
|
||||
from app.schemas import CurrentUser
|
||||
from app.services.audit import record_audit_event
|
||||
from app.services.data_quality_common import has_open_issue as _has_open_issue
|
||||
from app.services.data_quality_common import issue_due_at
|
||||
from app.services.data_quality_common import load_open_issue as _load_open_issue
|
||||
from app.services.data_quality_common import new_scan_ref as _new_scan_ref
|
||||
from app.services.data_quality_duplicate_scan import scan_duplicate_customers
|
||||
from app.services.data_quality_odometer import (
|
||||
open_odometer_regression_issue as open_odometer_regression_issue,
|
||||
)
|
||||
from app.services.data_quality_odometer import (
|
||||
resolve_odometer_regression as resolve_odometer_regression,
|
||||
)
|
||||
from app.services.vehicle_status import (
|
||||
RECOMMENDATION_CODE_NO_CONFLICT,
|
||||
VehicleStatusRecommendation,
|
||||
@@ -28,15 +40,6 @@ REQUIRED_VEHICLE_FIELDS = ("registration_number", "make", "model", "location")
|
||||
DATA_QUALITY_SCAN_LOCK_ID = 6_138_493_717_091_029_491
|
||||
|
||||
|
||||
def issue_due_at(detected_at: datetime, severity: str) -> datetime:
|
||||
"""Return the local operational SLA deadline for a newly detected issue."""
|
||||
return detected_at + {
|
||||
"high": timedelta(hours=4),
|
||||
"medium": timedelta(days=1),
|
||||
"low": timedelta(days=3),
|
||||
}.get(severity, timedelta(days=1))
|
||||
|
||||
|
||||
@dataclass
|
||||
class ScanResult:
|
||||
created: dict[str, int] = field(default_factory=dict)
|
||||
@@ -45,25 +48,6 @@ class ScanResult:
|
||||
self.created[rule_type] = self.created.get(rule_type, 0) + 1
|
||||
|
||||
|
||||
def _has_open_issue(db: Session, rule_type: str, entity_type: str, entity_id: uuid.UUID) -> bool:
|
||||
return (
|
||||
db.scalar(
|
||||
select(DataQualityIssue.id).where(
|
||||
DataQualityIssue.rule_type == rule_type,
|
||||
DataQualityIssue.entity_type == entity_type,
|
||||
DataQualityIssue.entity_id == entity_id,
|
||||
DataQualityIssue.status == "open",
|
||||
)
|
||||
)
|
||||
is not None
|
||||
)
|
||||
|
||||
|
||||
def _new_scan_ref(prefix: str) -> str:
|
||||
"""Generate a stable human-readable prefix with a concurrent-safe suffix."""
|
||||
return f"{prefix}-{uuid.uuid4().hex[:10].upper()}"
|
||||
|
||||
|
||||
def _open_issue(
|
||||
db: Session,
|
||||
scan: ScanResult,
|
||||
@@ -76,6 +60,7 @@ def _open_issue(
|
||||
entity_ref: str,
|
||||
related_refs: list[str],
|
||||
signals: list[dict] | None = None,
|
||||
evidence_extra: dict | None = None,
|
||||
) -> None:
|
||||
if _has_open_issue(db, rule_type, entity_type, entity_id):
|
||||
return
|
||||
@@ -103,6 +88,7 @@ def _open_issue(
|
||||
"related_refs": related_refs,
|
||||
"signals": signals or [],
|
||||
}
|
||||
evidence.update(evidence_extra or {})
|
||||
if previous is not None:
|
||||
evidence["reopened_from"] = previous.public_ref
|
||||
evidence["previous_decision"] = previous.status
|
||||
@@ -235,57 +221,110 @@ def _scan_vehicle_status_conflicts(db: Session, scan: ScanResult) -> None:
|
||||
)
|
||||
|
||||
|
||||
def _scan_odometer_regressions(db: Session, scan: ScanResult) -> None:
|
||||
# The seed dataset's vehicle.odometer_km is generated independently of booking
|
||||
# history, so comparing every historical booking against it produces near-universal
|
||||
# false positives. Instead check the booking sequence's own internal consistency:
|
||||
# each vehicle's completed bookings should show a non-decreasing odometer reading.
|
||||
vehicles = {v.id: v for v in db.scalars(select(Vehicle)).all()}
|
||||
bookings_by_vehicle: dict[uuid.UUID, list[Booking]] = {}
|
||||
for booking in db.scalars(
|
||||
def _scan_odometer_regressions(
|
||||
db: Session,
|
||||
scan: ScanResult,
|
||||
*,
|
||||
actor_label: str | None,
|
||||
actor_type: str,
|
||||
) -> None:
|
||||
# Compare the chronological history with itself rather than every historical reading
|
||||
# to today's canonical value. That detects imported checkout/return/maintenance
|
||||
# regressions without flagging every legitimate older reading.
|
||||
# Runtime checkout/return/maintenance mutations take the vehicle lock before they
|
||||
# inspect or append DQ-03 evidence. Taking the same lock here makes scan-vs-command
|
||||
# check/merge atomic and prevents a partial-unique race for the open issue.
|
||||
vehicles = {
|
||||
v.id: v for v in db.scalars(select(Vehicle).order_by(Vehicle.id).with_for_update()).all()
|
||||
}
|
||||
readings_by_vehicle: dict[uuid.UUID, list[tuple[datetime, str, int, str, str | None]]] = {}
|
||||
inspections = db.scalars(select(Inspection)).all()
|
||||
return_inspection_booking_ids = {
|
||||
inspection.booking_id for inspection in inspections if inspection.type == "return"
|
||||
}
|
||||
returned_bookings = db.scalars(
|
||||
select(Booking).where(Booking.status == "returned", Booking.end_odometer_km.is_not(None))
|
||||
).all():
|
||||
bookings_by_vehicle.setdefault(booking.vehicle_id, []).append(booking)
|
||||
).all()
|
||||
booking_ref_by_id = {booking.id: booking.public_ref for booking in returned_bookings}
|
||||
for booking in returned_bookings:
|
||||
# A real return inspection owns the actual reading timestamp. Using the booking's
|
||||
# planned ends_at as a duplicate second reading can make an early return appear to
|
||||
# go backwards after a newer real inspection. Keep booking data only as the legacy
|
||||
# import fallback when no return inspection exists.
|
||||
if booking.id in return_inspection_booking_ids:
|
||||
continue
|
||||
assert booking.end_odometer_km is not None
|
||||
readings_by_vehicle.setdefault(booking.vehicle_id, []).append(
|
||||
(
|
||||
booking.ends_at,
|
||||
booking.public_ref,
|
||||
booking.end_odometer_km,
|
||||
"booking",
|
||||
booking.public_ref,
|
||||
)
|
||||
)
|
||||
for inspection in inspections:
|
||||
readings_by_vehicle.setdefault(inspection.vehicle_id, []).append(
|
||||
(
|
||||
inspection.completed_at,
|
||||
inspection.public_ref,
|
||||
inspection.odometer_km,
|
||||
inspection.type,
|
||||
(
|
||||
booking_ref_by_id.get(inspection.booking_id)
|
||||
if inspection.type == "return"
|
||||
else None
|
||||
),
|
||||
)
|
||||
)
|
||||
for record in db.scalars(select(MaintenanceRecord)).all():
|
||||
readings_by_vehicle.setdefault(record.vehicle_id, []).append(
|
||||
(record.occurred_at, record.public_ref, record.odometer_km, "maintenance", None)
|
||||
)
|
||||
|
||||
for vehicle_id, bookings in bookings_by_vehicle.items():
|
||||
bookings.sort(key=lambda b: b.ends_at)
|
||||
for earlier, later in zip(bookings, bookings[1:], strict=False):
|
||||
# The query above filters end_odometer_km IS NOT NULL, so both are ints here.
|
||||
assert earlier.end_odometer_km is not None
|
||||
assert later.end_odometer_km is not None
|
||||
if later.end_odometer_km < earlier.end_odometer_km:
|
||||
for vehicle_id, readings in readings_by_vehicle.items():
|
||||
readings.sort(key=lambda reading: (reading[0], reading[1]))
|
||||
highest = readings[0] if readings else None
|
||||
for later in readings[1:]:
|
||||
assert highest is not None
|
||||
if later[2] < highest[2]:
|
||||
vehicle = vehicles[vehicle_id]
|
||||
_open_issue(
|
||||
had_open_issue = _has_open_issue(
|
||||
db,
|
||||
scan,
|
||||
rule_type="odometer_regression",
|
||||
entity_type="vehicle",
|
||||
entity_id=vehicle_id,
|
||||
severity="medium",
|
||||
summary=(
|
||||
f"Booking {later.public_ref} recorded {later.end_odometer_km} km, "
|
||||
f"below the {earlier.end_odometer_km} km recorded by earlier "
|
||||
f"booking {earlier.public_ref}."
|
||||
),
|
||||
entity_ref=vehicle.public_ref,
|
||||
related_refs=[earlier.public_ref, later.public_ref],
|
||||
signals=[
|
||||
{
|
||||
"code": "odometer.regression",
|
||||
"params": {
|
||||
"later_ref": later.public_ref,
|
||||
"later_km": later.end_odometer_km,
|
||||
"earlier_ref": earlier.public_ref,
|
||||
"earlier_km": earlier.end_odometer_km,
|
||||
},
|
||||
}
|
||||
],
|
||||
"odometer_regression",
|
||||
"vehicle",
|
||||
vehicle_id,
|
||||
)
|
||||
break
|
||||
issue = open_odometer_regression_issue(
|
||||
db,
|
||||
vehicle=vehicle,
|
||||
reading_ref=later[1],
|
||||
reading_km=later[2],
|
||||
canonical_km=highest[2],
|
||||
canonical_ref=highest[1],
|
||||
source_type=later[3],
|
||||
related_refs=list(
|
||||
dict.fromkeys(
|
||||
[highest[1], later[1], *([later[4]] if later[4] is not None else [])]
|
||||
)
|
||||
),
|
||||
correctable_booking_refs=[later[4]] if later[4] is not None else [],
|
||||
public_ref=_new_scan_ref("DQ-SCAN"),
|
||||
actor_label=actor_label,
|
||||
actor_type=actor_type,
|
||||
)
|
||||
if issue is not None and not had_open_issue:
|
||||
scan.bump("odometer_regression")
|
||||
if later[2] > highest[2]:
|
||||
highest = later
|
||||
|
||||
|
||||
def run_scan(
|
||||
db: Session, *, actor_label: str | None = None, actor_type: str = "user"
|
||||
db: Session,
|
||||
*,
|
||||
actor_label: str | None = None,
|
||||
actor_type: str = "user",
|
||||
commit: bool = True,
|
||||
) -> ScanResult:
|
||||
# The check-then-insert work below spans several rules. Serialise whole scans at the
|
||||
# database boundary so API and n8n triggers cannot both observe an empty condition.
|
||||
@@ -293,7 +332,12 @@ def run_scan(
|
||||
scan = ScanResult()
|
||||
scan_duplicate_customers(db, scan, _open_issue)
|
||||
_scan_missing_required_fields(db, scan)
|
||||
_scan_odometer_regressions(db, scan)
|
||||
_scan_odometer_regressions(
|
||||
db,
|
||||
scan,
|
||||
actor_label=actor_label,
|
||||
actor_type=actor_type,
|
||||
)
|
||||
_scan_booking_overlaps(db, scan)
|
||||
_scan_vehicle_status_conflicts(db, scan)
|
||||
if actor_label is not None:
|
||||
@@ -305,28 +349,13 @@ def run_scan(
|
||||
entity_type="system",
|
||||
metadata={"created": scan.created},
|
||||
)
|
||||
db.commit()
|
||||
if commit:
|
||||
db.commit()
|
||||
else:
|
||||
db.flush()
|
||||
return scan
|
||||
|
||||
|
||||
def _load_open_issue(
|
||||
db: Session, public_ref: str, *, lock: bool = True
|
||||
) -> DataQualityIssue:
|
||||
statement = select(DataQualityIssue).where(DataQualityIssue.public_ref == public_ref)
|
||||
if lock:
|
||||
statement = statement.with_for_update()
|
||||
issue = db.scalar(statement)
|
||||
if issue is None:
|
||||
raise AppError("ISSUE_NOT_FOUND", "Data quality issue not found.", status_code=404)
|
||||
if issue.status != "open":
|
||||
raise AppError(
|
||||
"ISSUE_NOT_OPEN",
|
||||
f"Issue is '{issue.status}', not 'open'.",
|
||||
status_code=409,
|
||||
)
|
||||
return issue
|
||||
|
||||
|
||||
def defer_issue(db: Session, public_ref: str, actor: CurrentUser) -> DataQualityIssue:
|
||||
issue = _load_open_issue(db, public_ref)
|
||||
issue.status = "deferred"
|
||||
@@ -456,115 +485,6 @@ def provide_missing_fields(
|
||||
return issue
|
||||
|
||||
|
||||
def resolve_odometer_regression(
|
||||
db: Session, public_ref: str, body: ResolveOdometerRegressionRequest, actor: CurrentUser
|
||||
) -> DataQualityIssue:
|
||||
issue = _load_open_issue(db, public_ref)
|
||||
if issue.rule_type != "odometer_regression":
|
||||
raise AppError(
|
||||
"NOT_AN_ODOMETER_ISSUE",
|
||||
"This issue is not an odometer_regression issue.",
|
||||
status_code=409,
|
||||
)
|
||||
# Lock order is booking -> vehicle everywhere (checkout, return, reschedule); taking
|
||||
# the vehicle lock first here would be a deadlock waiting to happen under concurrency.
|
||||
booking: Booking | None = None
|
||||
if body.decision != "retain_canonical":
|
||||
related_refs = issue.evidence_json.get("related_refs", [])
|
||||
if body.booking_ref not in related_refs:
|
||||
raise AppError(
|
||||
"INVALID_BOOKING_REFERENCE",
|
||||
"booking_ref must be one of this issue's related bookings.",
|
||||
status_code=422,
|
||||
)
|
||||
if body.corrected_odometer_km is None:
|
||||
raise AppError(
|
||||
"CORRECTED_VALUE_REQUIRED",
|
||||
"corrected_odometer_km is required when correcting a reading.",
|
||||
status_code=422,
|
||||
)
|
||||
booking = db.scalar(
|
||||
select(Booking).where(Booking.public_ref == body.booking_ref).with_for_update()
|
||||
)
|
||||
if booking is None:
|
||||
raise AppError(
|
||||
"BOOKING_NOT_FOUND", "The booking to correct was not found.", status_code=404
|
||||
)
|
||||
vehicle = db.scalar(select(Vehicle).where(Vehicle.id == issue.entity_id).with_for_update())
|
||||
if vehicle is None:
|
||||
raise AppError(
|
||||
"VEHICLE_NOT_FOUND", "The vehicle for this issue was not found.", status_code=404
|
||||
)
|
||||
|
||||
correlation_id = uuid.uuid4()
|
||||
|
||||
if body.decision == "retain_canonical":
|
||||
record_audit_event(
|
||||
db,
|
||||
actor_type="user",
|
||||
actor_label=actor.display_name,
|
||||
action="data_quality_odometer_retained",
|
||||
entity_type="vehicle",
|
||||
entity_id=vehicle.id,
|
||||
correlation_id=correlation_id,
|
||||
metadata={"issue_ref": issue.public_ref, "canonical_odometer_km": vehicle.odometer_km},
|
||||
)
|
||||
else:
|
||||
assert booking is not None and body.corrected_odometer_km is not None
|
||||
# Never silently lower the canonical odometer: a correction must be at or above
|
||||
# the current canonical value, otherwise it would just create a new regression.
|
||||
if body.corrected_odometer_km < vehicle.odometer_km:
|
||||
raise AppError(
|
||||
"CORRECTION_BELOW_CANONICAL",
|
||||
(
|
||||
f"Corrected value {body.corrected_odometer_km} km is still below the "
|
||||
f"canonical {vehicle.odometer_km} km; it would not resolve the regression."
|
||||
),
|
||||
status_code=422,
|
||||
)
|
||||
|
||||
before = {
|
||||
"booking_end_odometer_km": booking.end_odometer_km,
|
||||
"vehicle_odometer_km": vehicle.odometer_km,
|
||||
}
|
||||
booking.end_odometer_km = body.corrected_odometer_km
|
||||
vehicle.odometer_km = body.corrected_odometer_km
|
||||
vehicle.version += 1
|
||||
record_audit_event(
|
||||
db,
|
||||
actor_type="user",
|
||||
actor_label=actor.display_name,
|
||||
action="data_quality_odometer_corrected",
|
||||
entity_type="vehicle",
|
||||
entity_id=vehicle.id,
|
||||
correlation_id=correlation_id,
|
||||
before=before,
|
||||
after={
|
||||
"booking_end_odometer_km": booking.end_odometer_km,
|
||||
"vehicle_odometer_km": vehicle.odometer_km,
|
||||
},
|
||||
metadata={"issue_ref": issue.public_ref, "booking_ref": booking.public_ref},
|
||||
)
|
||||
|
||||
issue.status = "resolved"
|
||||
issue.resolved_at = datetime.now(UTC)
|
||||
issue.resolved_by = actor.display_name
|
||||
record_audit_event(
|
||||
db,
|
||||
actor_type="user",
|
||||
actor_label=actor.display_name,
|
||||
action="data_quality_issue_resolved",
|
||||
entity_type="data_quality_issue",
|
||||
entity_id=issue.id,
|
||||
correlation_id=correlation_id,
|
||||
before={"status": "open"},
|
||||
after={"status": "resolved"},
|
||||
metadata={"decision": body.decision, "note": body.note},
|
||||
)
|
||||
db.commit()
|
||||
return issue
|
||||
|
||||
|
||||
def resolve_booking_overlap(
|
||||
db: Session, public_ref: str, booking_ref: str, note: str | None, actor: CurrentUser
|
||||
) -> DataQualityIssue:
|
||||
|
||||
Reference in New Issue
Block a user