diff --git a/PROJECT_STATE.md b/PROJECT_STATE.md index 634a9d2..1e9420c 100644 --- a/PROJECT_STATE.md +++ b/PROJECT_STATE.md @@ -872,3 +872,119 @@ scenarios, demo manifest, About page). Gap audit: `docs/demo-release/current-dem test results, clean-checkout result, deployment evidence for both the feature branch and master, responsive/accessibility results, known limitations, rollback procedure) plus 10 screenshots in `artifacts/fleet-ops-release/screenshots/`. + +## Fleet Ops correction: safe status-recommendation flow, MO-016, message codes (2026-08-03) — IN PROGRESS on fix/fleet-ops-i18n-status-flow + +Branch `fix/fleet-ops-i18n-status-flow`, created from master's post-release head +(`18344bc`). Audit and rationale in `docs/fleet-ops-correction/` (gap audit, i18n +inventory, vehicle-status decision table). Not yet merged to master. + +- **Status-recommendation flow redesigned** per the brief: the old single opaque + "calculate and apply recommended status" action is replaced by a single shared, pure + evaluator (`backend/app/services/vehicle_status.py::evaluate_vehicle_status`) used + identically by the scanner, a new non-mutating preview endpoint + (`POST .../status-recommendation`), and a transactional apply endpoint + (`POST .../apply-recommended-status`) that locks the row, recomputes facts, rejects a + stale `recommendation_token` (optimistic concurrency), refuses unsafe/manual-review + recommendations, and re-validates post-write before resolving the issue. Frontend + `DataQualityIssueDetail.tsx` shows "Review recommendation" → a decision panel + (current/recommended status, why, evidence, consequences, localized in all 3 + languages) → an exact "Change status to " confirm action → result, with a + distinct "Manual review required" state offering no generic apply button. +- Fixed the real unsafe shortcut this evaluator exists to eliminate: "maintenance + + active booking" no longer auto-recommends "rented" (current status is itself now a + blocking fact), and "maintenance with nothing else wrong" no longer auto-clears to + "available" (no fact proves maintenance is actually finished — that release stays a + manual decision). +- **MO-016 order independence**: order independence does not mean "same final status + regardless of order" — resolving the booking overlap first genuinely removes the + conflict, correctly leaving nothing to apply. What must (and does) hold either way: + the recommendation always reflects real current facts, and nothing unsafe is ever + applied (never "rented"). Verified by both a backend test + (`test_mo_016_status_conflict_recommendation_is_order_independent`, explicitly scoped + to MO-016/DQ-DEMO-STATUS after finding the original version wasn't) and a browser-level + Playwright test in both orders. +- **"Fleet Ops" is a non-localizable brand constant** (`frontend/src/product.ts`, + backend `PRODUCT_NAME`), wired via `{{productName}}` interpolation everywhere the + brand appeared in locale prose; a permanent test fails the build if any locale file + ever defines the brand name or an `appName` key again. +- **Dynamic backend prose converted to message codes + params**: return status reasons, + audit field/actor-type labels, automation `last_error` (new `last_error_code` column, + migration `799d8800e241`), and search results (sections/vehicles/bookings/issues) all + now carry stable codes the frontend localizes; raw technical text is demoted to a + "Technical details" disclosure everywhere. +- **Knowledge-base fixes**: the demo provider's tokenizer silently dropped accented + characters (`[a-z0-9]+` split "véhicule" into "v"+"hicule"), breaking French + retrieval broadly; fixed to include the Latin-1 accented range. Also reweighted + section scoring so a body match (real substance) outranks a heading/title match (a + shallow structural hint) — the old weighting misranked the damage procedure behind + an unrelated document for the brief's exact validation question in all 3 languages. + Removed leftover "MobilityOps"/"PoC" mentions from 9 procedure documents (knowledge + prose is visible content, missed by the earlier rebrand). +- New `frontend/e2e/fleet-ops-correction.spec.ts` (14 tests) covers branding in 3 + languages, language persistence, the full status-recommendation flow (non-mutating + preview, exact confirm text, manual review, stale-token rejection), MO-016 order + independence, trilingual knowledge grounding, and localized audit/automation. Writing + it surfaced and fixed two real bugs: the frontend conflated "no conflict" with + "manual review required" (both carry `safe_to_apply: false`), and the original + MO-016 backend test never actually targeted MO-016's own issue. +- Added a keyboard/reduced-motion/no-color-only-status accessibility test for the new + status-decision panel; added `aria-live="polite"` to the panel so the applied + confirmation is announced. +- `contracts/openapi.yaml` and `README.md` updated: title is "Fleet Ops", the new + status-recommendation endpoint documented, apply-recommended-status's request body + and error codes documented, search endpoint's code+params shape documented, README + states the Fleet Ops/MobilityOps naming split explicitly and refreshes stale test + counts (151 backend, 108 Playwright). +- Gates green: 151 backend tests, Ruff, mypy, Alembic upgrade/downgrade verified, + frontend `tsc`/build, full 113-test Playwright suite (rebuilt `api`+`web` containers + each time before testing). +- Section 11D/E/F of the i18n test-strengthening brief done: a hardcoded-JSX-text + static check (`i18n-coverage.spec.ts`; had to anchor on backreferenced closing-tag + names — a naive `>text<` scan misread TypeScript generics like + `useState` as JSX spanning to the next unrelated `>`; verified against + both false positives and a deliberately-injected-then-reverted false negative), and a + 3-language route matrix (`fleet-ops-correction.spec.ts`) covering every main route: + no console errors, correct `html[lang]`, real page headings. +- **Clean-checkout drill (2026-08-03) — PASS.** Fresh `git clone --branch + fix/fleet-ops-i18n-status-flow` of only committed files into an isolated directory, + separate Compose project name and host ports (8129/1229/5679) so the working dev + stack was never touched. From empty volumes: `docker compose build` + `up -d` → + `alembic upgrade head` (lands on `799d8800e241`, the `last_error_code` migration) → + `reset_and_seed` (50 vehicles / 180 customers / 246 bookings / 27 data-quality issues + / 20 workflow runs — matches the corrected deterministic count) → 151 backend tests + + Ruff + mypy green → `npm ci` + frontend build green → full Playwright suite green + (113 tests; a few sequential-run-only flakes reproduced from resource contention of + running two full Docker stacks at once on one machine — every one confirmed to pass + in isolation, none touch code this branch changed) → final reset → + `scenario_integrity.all_ready: true`. Isolated stack, containers, volumes and images + torn down afterward; original dev environment confirmed untouched and reset to + baseline. +- **Unraid deployment (2026-08-03/04) — PASS.** Pushed `fix/fleet-ops-i18n-status-flow` + to origin, deployed via `git archive` → `scp` → extract into + `/mnt/user/appdata/mobilityops` (preserving `.env`) → `.deploy/source-revision` → + rebuild `api`+`web` → `alembic upgrade head` → reset/reseed, at + `http://192.168.10.150:1236`. **Live validation directly caught a real bug**: every + data-quality issue's top-of-page evidence summary was unconditionally showing raw + English (e.g. "vehicle marked available while reserved bookings conflict") in all + three languages, because the frontend never finished the evidence.signals + localization the backend had already been emitting. Fixed (commit `2e4fb43`): + DataQualityIssueDetail.tsx now renders `evidence.signals` through the operator's + locale as the primary text, raw text moved to "Technical details" only, the 4 + DQ-DEMO-* seed rows got real computed signals (the duplicate-customer similarity + score is the actual SequenceMatcher ratio on the seeded names), and a regression + test locks this in. Redeployed with the fix; live-verified via `read_page` that + DQ-DEMO-STATUS now shows "Dit voertuig heeft twee overlappende reserveringen..." + instead of the raw English sentence. Full 116-test Playwright suite green against + the live server (`MOBILITYOPS_PUBLIC_URL=http://192.168.10.150:1236`), no console + errors, no errors in `api`/`web` container logs, both containers healthy, final + reset done, `scenario_integrity.all_ready: true`. +- **Not yet done**: the final merge to master with + `artifacts/fleet-ops-correction/final-summary.md` evidence. Do not claim PASS on + this correction until that's done. +- Commits so far on this branch: `6deb955` (status flow + brand constant + message + codes), `e6539d1` (knowledge fixes), `ac4b163` (Playwright spec updates for the new + flow), `1fdd2b3` (new E2E coverage + 2 bug fixes), `1e40775` (accessibility test), + `a7ac5ed` (docs), `7851e80` (11D/11F i18n tests), `cda2c32` (clean-checkout + evidence), `2e4fb43` (evidence-summary localization fix, found live on Unraid). + Deployed commit: `2e4fb43f093bfbdb04c4f74eed1e6c6d9a03c069`. diff --git a/README.md b/README.md index 0a1b3cc..0c1a8c9 100644 --- a/README.md +++ b/README.md @@ -1,8 +1,15 @@ -# MobilityOps +# Fleet Ops **Connected operations for vehicle rental and service teams.** -MobilityOps is a working proof of concept for a fictitious mobility company. It combines vehicle and booking operations, a controlled vehicle-return workflow, data-quality review, RAGcore-backed internal knowledge, n8n orchestration and read-only tools published through ITWorx MCP Hub. +Fleet Ops is a working proof of concept for a fictitious mobility company. It combines vehicle and booking operations, a controlled vehicle-return workflow, data-quality review, RAGcore-backed internal knowledge, n8n orchestration and read-only tools published through ITWorx MCP Hub. + +**Naming:** "Fleet Ops" is the product's visible name everywhere in the UI, the demo +knowledge base, and this documentation. "MobilityOps" remains the technical +identifier only — the repository name, local directory, package/module names, Docker +Compose project, deployment directory, and database names. The UI is fully trilingual +(nl-BE default, en-GB, fr-BE); see `docs/fleet-ops-correction/` for the localization +architecture, the vehicle-status decision table, and the correction evidence. The web application uses the premium responsive **Control Rail** interface: a compact operations-first workspace with persisted readiness metrics, evidence-led exceptions, @@ -114,9 +121,9 @@ All defaults are configurable via `.env` (see `.env.example`). ## Quality gates ```bash -make test # backend: pytest (127 tests) +make test # backend: pytest (151 tests) make lint # backend: ruff + mypy (strict, zero errors) -make e2e # frontend: Playwright end-to-end (56 tests, live stack required) +make e2e # frontend: Playwright end-to-end (113 tests, live stack required) ``` Frontend build/typecheck: `cd frontend && npm run build` (`tsc -b && vite build`). diff --git a/backend/alembic/versions/799d8800e241_outbox_last_error_code.py b/backend/alembic/versions/799d8800e241_outbox_last_error_code.py new file mode 100644 index 0000000..755e05b --- /dev/null +++ b/backend/alembic/versions/799d8800e241_outbox_last_error_code.py @@ -0,0 +1,25 @@ +"""outbox last_error_code + +Revision ID: 799d8800e241 +Revises: e7b08389f47f +Create Date: 2026-08-03 10:00:00.000000 + +""" +from typing import Sequence, Union + +from alembic import op +import sqlalchemy as sa + +# revision identifiers, used by Alembic. +revision: str = '799d8800e241' +down_revision: Union[str, None] = 'e7b08389f47f' +branch_labels: Union[str, Sequence[str], None] = None +depends_on: Union[str, Sequence[str], None] = None + + +def upgrade() -> None: + op.add_column('outbox_events', sa.Column('last_error_code', sa.String(length=60), nullable=True)) + + +def downgrade() -> None: + op.drop_column('outbox_events', 'last_error_code') diff --git a/backend/app/api/routers/bookings.py b/backend/app/api/routers/bookings.py index 1dae99f..68afa27 100644 --- a/backend/app/api/routers/bookings.py +++ b/backend/app/api/routers/bookings.py @@ -89,6 +89,8 @@ def preview_return( resulting_odometer_km=evaluation.resulting_odometer_km, resulting_vehicle_status=evaluation.resulting_vehicle_status, status_reason=evaluation.status_reason, + status_reason_code=evaluation.status_reason_code, + status_reason_params=evaluation.status_reason_params, would_create_quality_issue=evaluation.would_create_quality_issue, attention_reasons=evaluation.attention_reasons, next_booking_risk=( diff --git a/backend/app/api/routers/dashboard.py b/backend/app/api/routers/dashboard.py index 3385bec..15039c8 100644 --- a/backend/app/api/routers/dashboard.py +++ b/backend/app/api/routers/dashboard.py @@ -108,6 +108,7 @@ def get_dashboard( status=r.delivery_status, attempts=r.attempts, last_error=r.last_error, + last_error_code=r.last_error_code, occurred_at=r.occurred_at, ) for r in recent diff --git a/backend/app/api/routers/data_quality.py b/backend/app/api/routers/data_quality.py index 52f3d8c..2daeed3 100644 --- a/backend/app/api/routers/data_quality.py +++ b/backend/app/api/routers/data_quality.py @@ -11,6 +11,7 @@ from app.models.data_quality import DataQualityIssue from app.models.inspection import Inspection from app.models.vehicle import Vehicle from app.schemas import ( + ApplyRecommendedStatusRequest, ApplyRecommendedStatusResult, CurrentUser, DataQualityIssueDetailOut, @@ -21,11 +22,14 @@ from app.schemas import ( ResolveOdometerRegressionRequest, ResolveOverlapRequest, ScanResultOut, + StatusRecommendationOut, + VehicleStatusFactsOut, ) from app.services.data_quality import ( apply_recommended_status, defer_issue, merge_customers, + preview_vehicle_status_recommendation, provide_missing_fields, reject_issue, resolve_booking_overlap, @@ -241,17 +245,43 @@ def resolve_overlap( return _to_out(issue) +@router.post( + "/issues/{public_ref}/status-recommendation", response_model=StatusRecommendationOut +) +def status_recommendation( + public_ref: str, + db: Session = Depends(get_db), + user: CurrentUser = Depends(require_operations_manager), +) -> StatusRecommendationOut: + """Non-mutating preview: computes the recommendation without changing anything, + resolving no issue and writing no audit event. Safe to call repeatedly.""" + _issue, _vehicle, recommendation, token = preview_vehicle_status_recommendation(db, public_ref) + return StatusRecommendationOut( + current_status=recommendation.current_status, + recommended_status=recommendation.recommended_status, + recommendation_code=recommendation.recommendation_code, + safe_to_apply=recommendation.safe_to_apply, + manual_review_required=recommendation.manual_review_required, + facts=VehicleStatusFactsOut(**recommendation.facts.as_dict()), + blocking_reasons=recommendation.blocking_reasons, + recommendation_token=token, + ) + + @router.post( "/issues/{public_ref}/apply-recommended-status", response_model=ApplyRecommendedStatusResult ) def apply_status( public_ref: str, + body: ApplyRecommendedStatusRequest, db: Session = Depends(get_db), user: CurrentUser = Depends(require_operations_manager), ) -> ApplyRecommendedStatusResult: - issue, applied_status, reason = apply_recommended_status(db, public_ref, user) + issue, applied_status, reason_code = apply_recommended_status( + db, public_ref, user, body.recommendation_token + ) return ApplyRecommendedStatusResult( - issue=_to_out(issue), applied_status=applied_status, reason=reason + issue=_to_out(issue), applied_status=applied_status, reason_code=reason_code ) diff --git a/backend/app/api/routers/search.py b/backend/app/api/routers/search.py index c0b2287..f8aaa52 100644 --- a/backend/app/api/routers/search.py +++ b/backend/app/api/routers/search.py @@ -12,53 +12,81 @@ from app.schemas import CurrentUser, SearchResponse, SearchResultItem router = APIRouter(prefix="/api/v1/search", tags=["search"]) -# Static application sections. Manager-only sections are filtered by role, mirroring the -# same nav visibility rule Layout.tsx applies -- search must never surface a destination -# the current role can't actually reach. +# Static application sections. `id` is a stable code matching navigation.json's +# `items.*` keys -- the frontend localizes both the section label and its one-line +# detail from `id`, so no English prose is sent over the wire (search.sections. in +# every locale; see docs/fleet-ops-correction/i18n-inventory.md). Manager-only sections +# are filtered by role, mirroring the same nav visibility rule Layout.tsx applies -- +# search must never surface a destination the current role can't actually reach. _SECTIONS: list[dict] = [ { - "label": "Overview", - "detail": "Operations dashboard", + "id": "overview", "link": "/dashboard", - "terms": ["overview", "dashboard", "readiness"], + # Search terms deliberately span all three supported UI languages (not just + # English) so a query never depends on the operator's selected locale. + "terms": ["overview", "dashboard", "readiness", "overzicht", "aperçu", "tableau de bord"], }, { - "label": "Fleet", - "detail": "Vehicle registry", + "id": "fleet", "link": "/vehicles", - "terms": ["fleet", "vehicle", "vehicles"], + "terms": ["fleet", "vehicle", "vehicles", "wagenpark", "voertuig", "flotte", "véhicule"], }, { - "label": "Bookings", - "detail": "Rental bookings", + "id": "bookings", "link": "/bookings", - "terms": ["booking", "bookings", "rental"], + "terms": [ + "booking", + "bookings", + "rental", + "boeking", + "boekingen", + "verhuur", + "réservation", + "réservations", + "location", + ], }, { - "label": "Data quality", - "detail": "Quality workbench", + "id": "quality", "link": "/data-quality", - "terms": ["quality", "data quality", "issues"], + "terms": [ + "quality", + "data quality", + "issues", + "kwaliteit", + "datakwaliteit", + "problemen", + "qualité", + "problèmes", + ], "role": "operations_manager", }, { - "label": "Knowledge", - "detail": "Procedure assistant", + "id": "knowledge", "link": "/knowledge", - "terms": ["knowledge", "procedures"], + "terms": ["knowledge", "procedures", "kennis", "procedures", "connaissances", "procédures"], }, { - "label": "Integrations", - "detail": "Automation and integration status", + "id": "integrations", "link": "/automation", - "terms": ["automation", "integrations", "systems", "n8n"], + "terms": [ + "automation", + "integrations", + "systems", + "n8n", + "automatisering", + "integraties", + "systemen", + "automatisation", + "intégrations", + "systèmes", + ], "role": "operations_manager", }, { - "label": "Audit trail", - "detail": "Audit history", + "id": "audit", "link": "/audit", - "terms": ["audit", "history"], + "terms": ["audit", "history", "geschiedenis", "historique"], "role": "operations_manager", }, ] @@ -83,8 +111,8 @@ def search( results.append( SearchResultItem( type="section", - label=section["label"], - detail=section["detail"], + label=section["id"], + detail_code=section["id"], link=section["link"], ) ) @@ -108,7 +136,8 @@ def search( SearchResultItem( type="vehicle", label=v.public_ref, - detail=f"{v.make} {v.model} · {v.location}", + detail_code="vehicleSummary", + detail_params={"make": v.make, "model": v.model, "location": v.location}, link=f"/vehicles/{v.public_ref}", ) ) @@ -120,7 +149,7 @@ def search( SearchResultItem( type="booking", label=b.public_ref, - detail=b.status, + detail_code=b.status, link=f"/bookings/{b.public_ref}", ) ) @@ -138,7 +167,7 @@ def search( SearchResultItem( type="data_quality_issue", label=i.public_ref, - detail=i.rule_type.replace("_", " "), + detail_code=i.rule_type, link=f"/data-quality/{i.public_ref}", ) ) diff --git a/backend/app/api/routers/workflows.py b/backend/app/api/routers/workflows.py index 76ce9c9..736fcec 100644 --- a/backend/app/api/routers/workflows.py +++ b/backend/app/api/routers/workflows.py @@ -23,6 +23,7 @@ def _to_out(event: OutboxEvent) -> AutomationRunOut: status=event.delivery_status, attempts=event.attempts, last_error=event.last_error, + last_error_code=event.last_error_code, occurred_at=event.occurred_at, ) diff --git a/backend/app/core/config.py b/backend/app/core/config.py index 62cb07a..fc2cd59 100644 --- a/backend/app/core/config.py +++ b/backend/app/core/config.py @@ -2,6 +2,12 @@ from functools import lru_cache from pydantic_settings import BaseSettings, SettingsConfigDict +# The visible product name is fixed and never translated or configured per-deployment -- +# see docs/fleet-ops-correction/current-gap-audit.md section 1. Internal identifiers +# (package name, Compose project, database name, repository) intentionally remain +# "mobilityops"; this constant is only for user-facing surfaces (e.g. the OpenAPI title). +PRODUCT_NAME = "Fleet Ops" + class Settings(BaseSettings): model_config = SettingsConfigDict(env_file=".env", extra="ignore") diff --git a/backend/app/main.py b/backend/app/main.py index 6d9d51e..a44449e 100644 --- a/backend/app/main.py +++ b/backend/app/main.py @@ -19,7 +19,7 @@ from app.api.routers import ( vehicles, workflows, ) -from app.core.config import get_settings +from app.core.config import PRODUCT_NAME, get_settings from app.core.errors import AppError, error_body from app.services.dispatcher import start_background_dispatcher, stop_background_dispatcher @@ -33,7 +33,7 @@ async def lifespan(_app: FastAPI): stop_background_dispatcher() -app = FastAPI(title="MobilityOps API", version="0.1.0", lifespan=lifespan) +app = FastAPI(title=f"{PRODUCT_NAME} API", version="0.1.0", lifespan=lifespan) app.add_middleware( CORSMiddleware, diff --git a/backend/app/models/outbox.py b/backend/app/models/outbox.py index 1d63a75..e3d2cf4 100644 --- a/backend/app/models/outbox.py +++ b/backend/app/models/outbox.py @@ -26,4 +26,9 @@ class OutboxEvent(TimestampMixin, Base): attempts: Mapped[int] = mapped_column(Integer, nullable=False, default=0) next_attempt_at: Mapped[datetime | None] = mapped_column(DateTime(timezone=True)) last_error: Mapped[str | None] = mapped_column(Text) + # Stable, localizable classification of last_error -- the frontend renders a + # localized summary from this code as the primary text and shows last_error itself + # only under "Technical details" (section 10 of docs/fleet-ops-correction/ + # current-gap-audit.md). Kept alongside the raw message for backward compatibility. + last_error_code: Mapped[str | None] = mapped_column(String(60)) external_run_id: Mapped[str | None] = mapped_column(String(120)) diff --git a/backend/app/schemas.py b/backend/app/schemas.py index 00c8bfc..5c2ef51 100644 --- a/backend/app/schemas.py +++ b/backend/app/schemas.py @@ -83,6 +83,8 @@ class ReturnPreviewResult(BaseModel): resulting_odometer_km: int resulting_vehicle_status: str status_reason: str + status_reason_code: str + status_reason_params: dict[str, str | int] = {} would_create_quality_issue: bool attention_reasons: list[str] next_booking_risk: NextBookingRisk | None @@ -157,16 +159,41 @@ class ResolveOverlapRequest(BaseModel): note: str | None = Field(default=None, max_length=500) +class VehicleStatusFactsOut(BaseModel): + active_booking_refs: list[str] + overlapping_booking_pairs: list[list[str]] + service_threshold_reached: bool + odometer_km: int + next_service_km: int + open_booking_overlap_issue_ref: str | None = None + + +class StatusRecommendationOut(BaseModel): + current_status: str + recommended_status: str | None + recommendation_code: str + safe_to_apply: bool + manual_review_required: bool + facts: VehicleStatusFactsOut + blocking_reasons: list[str] + recommendation_token: str + + +class ApplyRecommendedStatusRequest(BaseModel): + recommendation_token: str + + class ApplyRecommendedStatusResult(BaseModel): issue: DataQualityIssueOut applied_status: str - reason: str + reason_code: str class SearchResultItem(BaseModel): type: Literal["vehicle", "booking", "data_quality_issue", "section"] label: str - detail: str + detail_code: str + detail_params: dict[str, str] = {} link: str @@ -269,6 +296,7 @@ class AutomationRunOut(BaseModel): status: str attempts: int last_error: str | None + last_error_code: str | None occurred_at: datetime diff --git a/backend/app/seed_loader.py b/backend/app/seed_loader.py index 938d219..1036519 100644 --- a/backend/app/seed_loader.py +++ b/backend/app/seed_loader.py @@ -4,6 +4,7 @@ import csv import uuid from dataclasses import dataclass from datetime import UTC, date, datetime, timedelta +from difflib import SequenceMatcher from pathlib import Path from sqlalchemy import delete, insert, update @@ -108,21 +109,22 @@ def load_seed(db: Session) -> SeedResult: customer_id_by_ref: dict[str, uuid.UUID] = {} customer_rows = [] + customer_row_by_ref: dict[str, dict] = {} for row in _read_csv("customers.csv"): cid = uuid.uuid4() customer_id_by_ref[row["public_ref"]] = cid - customer_rows.append( - { - "id": cid, - "public_ref": row["public_ref"], - "first_name": row["first_name"], - "last_name": row["last_name"], - "email": row["email"] or None, - "phone": row["phone"] or None, - "postal_code": row["postal_code"] or None, - "city": row["city"] or None, - } - ) + customer_row = { + "id": cid, + "public_ref": row["public_ref"], + "first_name": row["first_name"], + "last_name": row["last_name"], + "email": row["email"] or None, + "phone": row["phone"] or None, + "postal_code": row["postal_code"] or None, + "city": row["city"] or None, + } + customer_rows.append(customer_row) + customer_row_by_ref[row["public_ref"]] = customer_row db.execute(insert(Customer), customer_rows) counts["customers"] = len(customer_rows) # Second pass for merged_into (self-referencing FK) since target must exist first. @@ -223,11 +225,42 @@ def load_seed(db: Session) -> SeedResult: return "customer", customer_id_by_ref[entity_ref] return "vehicle", vehicle_id_by_ref[entity_ref] + def _seed_signals(public_ref: str, entity_ref: str, related_refs: list[str]) -> list[dict]: + # The four named DQ-DEMO-* rows anchor the guided demo's scripted scenarios, so + # they carry real, accurate structured signals (not just a legacy English + # sentence) -- the frontend renders these as the primary, localized evidence; + # see docs/fleet-ops-correction/current-gap-audit.md §6. + if public_ref == "DQ-DEMO-DUPLICATE": + a = customer_row_by_ref[entity_ref] + b = customer_row_by_ref[related_refs[0]] + name_a = f"{a['first_name']} {a['last_name']}".strip().lower() + name_b = f"{b['first_name']} {b['last_name']}".strip().lower() + ratio = SequenceMatcher(None, name_a, name_b).ratio() + return [ + {"code": "duplicate.exact_email"}, + {"code": "duplicate.exact_phone"}, + {"code": "duplicate.same_postal_code"}, + {"code": "duplicate.similar_name", "params": {"score": round(ratio, 2)}}, + ] + if public_ref == "DQ-DEMO-OVERLAP": + return [{"code": "overlap.reserved_bookings", "params": {"refs": related_refs}}] + if public_ref == "DQ-DEMO-STATUS": + return [{"code": "vehicle.booking_conflict"}] + if public_ref == "DQ-DEMO-ATTENTION": + return [ + { + "code": "attention.upcoming_booking_missing_inspection", + "params": {"booking_ref": related_refs[0] if related_refs else ""}, + } + ] + return [] + dq_rows = [] now = datetime.now(UTC) for row in _read_csv("data_quality_issues.csv"): entity_type, entity_id = resolve_entity(row["entity_ref"]) related_ref = row.get("related_ref") or "" + related_refs = related_ref.split("|") if related_ref else [] dq_rows.append( { "id": uuid.uuid4(), @@ -240,7 +273,8 @@ def load_seed(db: Session) -> SeedResult: "evidence_json": { "summary": row["evidence"], "entity_ref": row["entity_ref"], - "related_refs": related_ref.split("|") if related_ref else [], + "related_refs": related_refs, + "signals": _seed_signals(row["public_ref"], row["entity_ref"], related_refs), }, "proposed_action_json": {}, "detected_at": now, @@ -287,6 +321,9 @@ def load_seed(db: Session) -> SeedResult: "attempts": int(row["attempts"]), "next_attempt_at": None, "last_error": row["last_error"] or None, + # The seed dataset's one synthetic failure (BK-H-0020) models a + # connection-timeout-style delivery failure -- see workflow_runs.csv. + "last_error_code": "connectionError" if row["last_error"] else None, "external_run_id": None, } ) diff --git a/backend/app/services/data_quality.py b/backend/app/services/data_quality.py index e8610d8..c2077ff 100644 --- a/backend/app/services/data_quality.py +++ b/backend/app/services/data_quality.py @@ -15,6 +15,13 @@ from app.models.data_quality import DataQualityIssue from app.models.vehicle import Vehicle from app.schemas import CurrentUser, ResolveOdometerRegressionRequest from app.services.audit import record_audit_event +from app.services.vehicle_status import ( + RECOMMENDATION_CODE_NO_CONFLICT, + VehicleStatusRecommendation, + compute_recommendation_token, + evaluate_vehicle_status, + gather_vehicle_status_facts, +) REQUIRED_CUSTOMER_FIELDS = ("first_name", "last_name") REQUIRED_VEHICLE_FIELDS = ("registration_number", "make", "model", "location") @@ -69,6 +76,7 @@ def _open_issue( summary: str, entity_ref: str, related_refs: list[str], + signals: list[dict] | None = None, ) -> None: if _has_open_issue(db, rule_type, entity_type, entity_id): return @@ -87,10 +95,14 @@ def _open_issue( ) .order_by(DataQualityIssue.detected_at.desc()) ) + # `summary` is kept as a technical-fallback string (shown only under "Technical + # details"); `signals` is the stable, localizable structure the frontend renders as + # the primary evidence -- see docs/fleet-ops-correction/current-gap-audit.md §2/§6. evidence: dict = { "summary": summary, "entity_ref": entity_ref, "related_refs": related_refs, + "signals": signals or [], } if previous is not None: evidence["reopened_from"] = previous.public_ref @@ -121,22 +133,29 @@ def _scan_duplicate_customers(db: Session, scan: ScanResult) -> None: for i, a in enumerate(customers): for b in customers[i + 1 :]: score = 0 - signals = [] + signals: list[dict] = [] + summary_parts: list[str] = [] if _normalize(a.email) and _normalize(a.email) == _normalize(b.email): score += 60 - signals.append("exact email") + signals.append({"code": "duplicate.exact_email"}) + summary_parts.append("exact email") if _normalize(a.phone) and _normalize(a.phone) == _normalize(b.phone): score += 50 - signals.append("exact phone") + signals.append({"code": "duplicate.exact_phone"}) + summary_parts.append("exact phone") if _normalize(a.postal_code) and _normalize(a.postal_code) == _normalize(b.postal_code): score += 10 - signals.append("exact postal code") + signals.append({"code": "duplicate.same_postal_code"}) + summary_parts.append("exact postal code") name_a = f"{_normalize(a.first_name)} {_normalize(a.last_name)}" name_b = f"{_normalize(b.first_name)} {_normalize(b.last_name)}" ratio = SequenceMatcher(None, name_a, name_b).ratio() if ratio >= 0.5: score += round(ratio * 30) - signals.append("similar name") + signals.append( + {"code": "duplicate.similar_name", "params": {"score": round(ratio, 2)}} + ) + summary_parts.append("similar name") if score >= DUPLICATE_THRESHOLD: _open_issue( @@ -146,9 +165,10 @@ def _scan_duplicate_customers(db: Session, scan: ScanResult) -> None: entity_type="customer", entity_id=a.id, severity="high", - summary="; ".join(signals) + f" (score {score})", + summary="; ".join(summary_parts) + f" (score {score})", entity_ref=a.public_ref, related_refs=[b.public_ref], + signals=signals, ) @@ -170,6 +190,7 @@ def _scan_missing_required_fields(db: Session, scan: ScanResult) -> None: summary=f"Missing: {', '.join(missing)}", entity_ref=customer.public_ref, related_refs=[], + signals=[{"code": "missing_field", "params": {"field": f}} for f in missing], ) for vehicle in db.scalars(select(Vehicle).where(Vehicle.active.is_(True))).all(): @@ -185,6 +206,7 @@ def _scan_missing_required_fields(db: Session, scan: ScanResult) -> None: summary=f"Missing: {', '.join(missing)}", entity_ref=vehicle.public_ref, related_refs=[], + signals=[{"code": "missing_field", "params": {"field": f}} for f in missing], ) @@ -213,50 +235,47 @@ def _scan_booking_overlaps(db: Session, scan: ScanResult) -> None: summary=f"Overlapping bookings {first.public_ref} and {second.public_ref}", entity_ref=vehicle.public_ref, related_refs=[first.public_ref, second.public_ref], + signals=[ + { + "code": "overlap.reserved_bookings", + "params": {"refs": [first.public_ref, second.public_ref]}, + } + ], ) def _scan_vehicle_status_conflicts(db: Session, scan: ScanResult) -> None: + # Uses the same shared evaluator as the preview/apply flow (app.services.vehicle_status) + # so detection and resolution can never structurally disagree -- see + # docs/fleet-ops-correction/vehicle-status-decision-table.md. vehicles = db.scalars(select(Vehicle)).all() - active_by_vehicle: dict[uuid.UUID, list[Booking]] = {} - for booking in db.scalars(select(Booking).where(Booking.status == "active")).all(): - active_by_vehicle.setdefault(booking.vehicle_id, []).append(booking) - - open_high_by_vehicle = { - row[0] - for row in db.execute( - select(DataQualityIssue.entity_id).where( - DataQualityIssue.entity_type == "vehicle", - DataQualityIssue.status == "open", - DataQualityIssue.severity == "high", - ) - ).all() - } - for vehicle in vehicles: - has_active_booking = vehicle.id in active_by_vehicle - reason = None - if vehicle.operational_status == "available" and has_active_booking: - reason = "marked available while an active booking exists" - elif vehicle.operational_status == "rented" and not has_active_booking: - reason = "marked rented without an active booking" - elif vehicle.operational_status == "available" and vehicle.id in open_high_by_vehicle: - reason = "marked available while a high-severity quality issue is open" - elif vehicle.operational_status == "maintenance" and has_active_booking: - reason = "marked maintenance while an active booking exists" + facts = gather_vehicle_status_facts(db, vehicle) + recommendation = evaluate_vehicle_status(vehicle, facts) + if recommendation.recommendation_code == RECOMMENDATION_CODE_NO_CONFLICT: + continue - if reason: - _open_issue( - db, - scan, - rule_type="vehicle_status_conflict", - entity_type="vehicle", - entity_id=vehicle.id, - severity="high", - summary=f"Vehicle {reason}", - entity_ref=vehicle.public_ref, - related_refs=[], - ) + signals = [{"code": recommendation.recommendation_code, "params": facts.as_dict()}] + summary = ( + f"Recommended status: {recommendation.recommended_status}" + if recommendation.recommended_status + else "Manual review required: active rental conflicts with a blocking condition" + ) + _open_issue( + db, + scan, + rule_type="vehicle_status_conflict", + entity_type="vehicle", + entity_id=vehicle.id, + severity="high", + summary=summary, + entity_ref=vehicle.public_ref, + related_refs=[ + *facts.active_booking_refs, + *(ref for pair in facts.overlapping_booking_pairs for ref in pair), + ], + signals=signals, + ) def _scan_odometer_regressions(db: Session, scan: ScanResult) -> None: @@ -295,6 +314,17 @@ def _scan_odometer_regressions(db: Session, scan: ScanResult) -> None: ), 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, + }, + } + ], ) break @@ -650,28 +680,7 @@ def resolve_booking_overlap( return issue -def _recommend_vehicle_status( - operational_status: str, has_active_booking: bool, has_open_high_issue: bool -) -> tuple[str, str] | None: - """The single authoritative recommendation function for vehicle_status_conflict, - mirroring the exact conditions `_scan_vehicle_status_conflicts` flags.""" - if operational_status == "available" and has_active_booking: - return "rented", "An active booking exists; the vehicle should be marked rented." - if operational_status == "rented" and not has_active_booking: - return "available", "No active booking exists; the vehicle should be marked available." - if operational_status == "available" and has_open_high_issue: - return "blocked", "A high-severity quality issue is open; the vehicle should be blocked." - if operational_status == "maintenance" and has_active_booking: - return ( - "rented", - "An active booking exists despite the maintenance status; it should be rented.", - ) - return None - - -def apply_recommended_status( - db: Session, public_ref: str, actor: CurrentUser -) -> tuple[DataQualityIssue, str, str]: +def _load_vehicle_status_conflict_issue(db: Session, public_ref: str) -> DataQualityIssue: issue = _load_open_issue(db, public_ref) if issue.rule_type != "vehicle_status_conflict": raise AppError( @@ -679,47 +688,77 @@ def apply_recommended_status( "This issue is not a vehicle_status_conflict issue.", status_code=409, ) + return issue + + +def preview_vehicle_status_recommendation( + db: Session, public_ref: str +) -> tuple[DataQualityIssue, Vehicle, VehicleStatusRecommendation, str]: + """Non-mutating: computes and returns the recommendation only. Never resolves the + issue, never writes an audit event, never queues automation -- safe to call as often + as the UI needs (e.g. every time the panel is opened) with zero side effects.""" + issue = _load_vehicle_status_conflict_issue(db, public_ref) + vehicle = db.scalar(select(Vehicle).where(Vehicle.id == issue.entity_id)) + if vehicle is None: + raise AppError( + "VEHICLE_NOT_FOUND", "The vehicle for this issue was not found.", status_code=404 + ) + facts = gather_vehicle_status_facts(db, vehicle, exclude_issue_id=issue.id) + recommendation = evaluate_vehicle_status(vehicle, facts) + token = compute_recommendation_token(vehicle, facts) + return issue, vehicle, recommendation, token + + +def apply_recommended_status( + db: Session, public_ref: str, actor: CurrentUser, expected_token: str +) -> tuple[DataQualityIssue, str, str]: + issue = _load_vehicle_status_conflict_issue(db, public_ref) + # Lock the vehicle row for the remainder of this transaction so a concurrent apply + # (or return/checkout) can't race between our fact-gathering and the write below. 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 ) - has_active_booking = ( - db.scalar( - select(Booking.id).where(Booking.vehicle_id == vehicle.id, Booking.status == "active") + facts = gather_vehicle_status_facts(db, vehicle, exclude_issue_id=issue.id) + recommendation = evaluate_vehicle_status(vehicle, facts) + current_token = compute_recommendation_token(vehicle, facts) + + if current_token != expected_token: + raise AppError( + "RECOMMENDATION_STALE", + "The underlying facts changed since this recommendation was shown; " + "review the recommendation again before applying it.", + status_code=409, ) - is not None - ) - has_open_high_issue = ( - db.scalar( - select(DataQualityIssue.id).where( - DataQualityIssue.entity_type == "vehicle", - DataQualityIssue.entity_id == vehicle.id, - DataQualityIssue.status == "open", - DataQualityIssue.severity == "high", - DataQualityIssue.id != issue.id, - ) + if recommendation.manual_review_required or not recommendation.safe_to_apply: + raise AppError( + "MANUAL_REVIEW_REQUIRED", + "This vehicle's state requires manual review; no automatic status change is safe.", + status_code=409, ) - is not None - ) - recommendation = _recommend_vehicle_status( - vehicle.operational_status, has_active_booking, has_open_high_issue - ) - if recommendation is None: + if recommendation.recommended_status is None: raise AppError( "NO_CONFLICT_DETECTED", "The current vehicle state no longer conflicts; nothing to apply.", status_code=409, ) - new_status, reason = recommendation + new_status = recommendation.recommended_status + reason_code = recommendation.recommendation_code before = {"operational_status": vehicle.operational_status} vehicle.operational_status = new_status vehicle.version += 1 - # Re-validate: the same recommendation function must find no further conflict. - if _recommend_vehicle_status(new_status, has_active_booking, has_open_high_issue) is not None: + # Re-validate against the same shared evaluator, over freshly-gathered facts, that + # applying this change actually leaves no conflict -- never trust the pre-computed + # recommendation alone for the post-condition. + post_facts = gather_vehicle_status_facts(db, vehicle, exclude_issue_id=issue.id) + post_check = evaluate_vehicle_status(vehicle, post_facts) + if post_check.recommendation_code not in ( + RECOMMENDATION_CODE_NO_CONFLICT, + ): raise AppError( "CONFLICT_STILL_PRESENT", "Applying the recommended status did not resolve the conflict.", @@ -737,7 +776,7 @@ def apply_recommended_status( correlation_id=correlation_id, before=before, after={"operational_status": vehicle.operational_status}, - metadata={"issue_ref": issue.public_ref, "reason": reason}, + metadata={"issue_ref": issue.public_ref, "reason_code": reason_code}, ) issue.status = "resolved" @@ -755,7 +794,7 @@ def apply_recommended_status( after={"status": "resolved"}, ) db.commit() - return issue, new_status, reason + return issue, new_status, reason_code MERGEABLE_FIELDS = ("first_name", "last_name", "email", "phone", "postal_code", "city") diff --git a/backend/app/services/dispatcher.py b/backend/app/services/dispatcher.py index c401002..66ff1b5 100644 --- a/backend/app/services/dispatcher.py +++ b/backend/app/services/dispatcher.py @@ -48,6 +48,7 @@ def _reclaim_stale_deliveries(batch_size: int = 10) -> int: f"(no outcome recorded within {settings.n8n_delivery_lease_seconds:.0f}s; " f"the process likely crashed mid-delivery). attempts preserved at {row.attempts}." )[:2000] + row.last_error_code = "staleLeaseRecovered" db.commit() return len(rows) finally: @@ -111,8 +112,10 @@ def _deliver_one(event_id: uuid.UUID) -> None: finally: db.close() + error_code: str | None if wire_event is None: success, error, body = False, payload_error, None + error_code = "malformedPayload" else: try: response = httpx.post( @@ -124,9 +127,11 @@ def _deliver_one(event_id: uuid.UUID) -> None: body = response.json() success = bool(body.get("ok", True)) error = None if success else f"n8n reported failure: {body}" + error_code = None if success else "remoteReportedFailure" except httpx.HTTPError as exc: success = False error = f"{type(exc).__name__}: {exc}" + error_code = "connectionError" body = None db = SessionLocal() @@ -138,10 +143,12 @@ def _deliver_one(event_id: uuid.UUID) -> None: if success: event.delivery_status = "succeeded" event.last_error = None + event.last_error_code = None event.next_attempt_at = None event.external_run_id = str((body or {}).get("event_id", event_id)) else: event.last_error = (error or "delivery failed")[:2000] + event.last_error_code = error_code or "unknownError" if event.attempts >= settings.n8n_max_attempts: event.delivery_status = "failed" event.next_attempt_at = None diff --git a/backend/app/services/knowledge/demo.py b/backend/app/services/knowledge/demo.py index 58e9433..0e4aa16 100644 --- a/backend/app/services/knowledge/demo.py +++ b/backend/app/services/knowledge/demo.py @@ -35,7 +35,11 @@ STOPWORDS_BY_LANGUAGE: dict[str, set[str]] = { }, } -_WORD_RE = re.compile(r"[a-z0-9]+") +# Includes the Latin-1 accented-letter range (à-ö, ø-ÿ) so French/Dutch words with +# diacritics (véhicule, réservation, geëscaleerd) tokenize as one word instead of +# splitting apart at the accented character -- a plain [a-z0-9]+ pattern silently +# drops every accent and fragments the word either side of it. +_WORD_RE = re.compile(r"[a-zà-öø-ÿ0-9]+") def _stem(word: str) -> str: @@ -218,17 +222,28 @@ class DemoKnowledgeProvider: def _score( self, query_tokens: set[str], section: ScoredSection, idf: dict[str, float] ) -> float: + # The section body is the strongest relevance signal -- it's the actual + # substance a heading or title can only hint at -- so a body match is weighted + # *above* heading/title matches, not below them. The previous 3x/2x/1x + # (heading/title/body) ordering let a single generic word in a heading (e.g. + # "vehicle", present in nearly every section) or a document's own title + # outrank a section whose body genuinely covers multiple, more distinctive + # query terms -- confirmed to misrank the brief's exact validation question in + # every one of the three languages (see docs/fleet-ops-correction/ + # current-gap-audit.md and i18n-inventory.md): nl-BE picked a checkout section + # over the damage procedure, en-GB and fr-BE picked the return procedure over + # the damage procedure, purely from heading/title overlap on common words. score = 0.0 for token in query_tokens: token_idf = idf.get(token, 0.0) if token_idf == 0.0: continue - if token in section.heading_tokens: + if token in section.body_tokens: score += 3 * token_idf - elif token in section.document.title_tokens: + elif token in section.heading_tokens: score += 2 * token_idf - elif token in section.body_tokens: - score += token_idf + elif token in section.document.title_tokens: + score += 1.5 * token_idf return score def ask( diff --git a/backend/app/services/returns.py b/backend/app/services/returns.py index 0f2bcf8..b5e578b 100644 --- a/backend/app/services/returns.py +++ b/backend/app/services/returns.py @@ -28,19 +28,41 @@ def _next_public_ref(db: Session) -> str: def _derive_vehicle_status_with_reason( body: RegisterReturnRequest, vehicle: Vehicle, new_odometer: int -) -> tuple[str, str]: +) -> tuple[str, str, dict[str, str | int]]: + # Stable, localizable codes + params -- the backend never emits prose here. The + # frontend renders review.reasonCodes. in the selected locale; the mirrored + # raw-English fallback strings live only in status_reason (shown under "Technical + # details") for backward compatibility. See docs/fleet-ops-correction/i18n-inventory.md. if body.damage_reported and body.technical_warning: - return "blocked", "Damage and a technical warning were both reported on return." + return ( + "blocked", + "returnBlockedDamageAndTechnical", + {}, + ) if body.damage_reported: - return "blocked", "Damage was reported on return." + return "blocked", "returnBlockedDamage", {} if body.technical_warning: - return "blocked", "A technical warning was reported on return." + return "blocked", "returnBlockedTechnicalWarning", {} if new_odometer >= vehicle.next_service_km: return ( "maintenance", - f"Odometer reached the {vehicle.next_service_km:,} km service threshold.", + "returnServiceThresholdReached", + {"threshold_km": vehicle.next_service_km}, ) - return "cleaning", "No damage, technical warning or service threshold; routed to cleaning." + return "cleaning", "returnRoutedToCleaning", {} + + +_STATUS_REASON_FALLBACK_TEXT: dict[str, str] = { + "returnBlockedDamageAndTechnical": ( + "Damage and a technical warning were both reported on return." + ), + "returnBlockedDamage": "Damage was reported on return.", + "returnBlockedTechnicalWarning": "A technical warning was reported on return.", + "returnServiceThresholdReached": "Odometer reached the service threshold.", + "returnRoutedToCleaning": ( + "No damage, technical warning or service threshold; routed to cleaning." + ), +} @dataclass @@ -51,6 +73,8 @@ class ReturnEvaluation: resulting_odometer_km: int resulting_vehicle_status: str status_reason: str + status_reason_code: str + status_reason_params: dict[str, str | int] would_create_quality_issue: bool attention_reasons: list[str] next_booking_risk: dict | None @@ -64,9 +88,10 @@ def evaluate_return( preview and commit can never drift apart.""" odometer_regression = body.end_odometer_km < vehicle.odometer_km resulting_odometer_km = vehicle.odometer_km if odometer_regression else body.end_odometer_km - resulting_status, status_reason = _derive_vehicle_status_with_reason( + resulting_status, status_reason_code, status_reason_params = _derive_vehicle_status_with_reason( body, vehicle, resulting_odometer_km ) + status_reason = _STATUS_REASON_FALLBACK_TEXT[status_reason_code] attention_reasons = [] if body.damage_reported: @@ -101,6 +126,8 @@ def evaluate_return( resulting_odometer_km=resulting_odometer_km, resulting_vehicle_status=resulting_status, status_reason=status_reason, + status_reason_code=status_reason_code, + status_reason_params=status_reason_params, would_create_quality_issue=odometer_regression, attention_reasons=attention_reasons, next_booking_risk=next_booking_risk, diff --git a/backend/app/services/vehicle_status.py b/backend/app/services/vehicle_status.py new file mode 100644 index 0000000..8eba07e --- /dev/null +++ b/backend/app/services/vehicle_status.py @@ -0,0 +1,218 @@ +"""The single authoritative vehicle-status evaluator. + +Used by the data-quality scanner (detection), the status-recommendation preview +endpoint, the apply endpoint, and tests -- so scan-time detection and resolve-time +recommendation can never structurally disagree (see docs/fleet-ops-correction/ +vehicle-status-decision-table.md for the full decision table and rationale). + +The evaluator only ever reasons from real, freshly-queried domain facts (an actually +active rental, a real service-threshold breach, a real overlapping-booking conflict) -- +never from a proxy like "does some other high-severity issue happen to be open". It is +therefore also order-independent: resolving, deferring or rejecting an unrelated issue on +the same vehicle never changes what this function returns, because it never looks at +issue history, only at the vehicle's/bookings' current state. +""" + +from __future__ import annotations + +import hashlib +import json +import uuid +from dataclasses import dataclass, field + +from sqlalchemy import select +from sqlalchemy.orm import Session + +from app.models.booking import Booking +from app.models.data_quality import DataQualityIssue +from app.models.vehicle import Vehicle + +# Every code below is a stable, localizable identifier -- see +# frontend/src/i18n/messageCodes.ts and quality:statusRecommendation.codes.* for the +# human-language mapping in all three supported locales. The backend never emits prose. +RECOMMENDATION_CODE_ACTIVE_RENTAL = "vehicle.active_rental" +RECOMMENDATION_CODE_SERVICE_THRESHOLD = "vehicle.service_threshold_reached" +RECOMMENDATION_CODE_BOOKING_CONFLICT = "vehicle.booking_conflict" +RECOMMENDATION_CODE_RENTAL_ENDED = "vehicle.rental_ended" +RECOMMENDATION_CODE_MANUAL_REVIEW = "vehicle.manual_review_required" +RECOMMENDATION_CODE_NO_CONFLICT = "vehicle.no_conflict" + + +@dataclass +class VehicleStatusFacts: + active_booking_refs: list[str] = field(default_factory=list) + overlapping_booking_pairs: list[tuple[str, str]] = field(default_factory=list) + service_threshold_reached: bool = False + odometer_km: int = 0 + next_service_km: int = 0 + open_booking_overlap_issue_ref: str | None = None + + @property + def has_active_rental(self) -> bool: + return len(self.active_booking_refs) > 0 + + @property + def has_booking_conflict(self) -> bool: + return ( + len(self.overlapping_booking_pairs) > 0 + or self.open_booking_overlap_issue_ref is not None + ) + + def as_dict(self) -> dict: + return { + "active_booking_refs": self.active_booking_refs, + "overlapping_booking_pairs": [list(pair) for pair in self.overlapping_booking_pairs], + "service_threshold_reached": self.service_threshold_reached, + "odometer_km": self.odometer_km, + "next_service_km": self.next_service_km, + "open_booking_overlap_issue_ref": self.open_booking_overlap_issue_ref, + } + + +@dataclass +class VehicleStatusRecommendation: + current_status: str + recommended_status: str | None + recommendation_code: str + safe_to_apply: bool + manual_review_required: bool + facts: VehicleStatusFacts + blocking_reasons: list[str] + + +def _overlapping_booking_pairs(bookings: list[Booking]) -> list[tuple[Booking, Booking]]: + ordered = sorted(bookings, key=lambda b: b.starts_at) + pairs: list[tuple[Booking, Booking]] = [] + for i, first in enumerate(ordered): + for second in ordered[i + 1 :]: + if second.starts_at < first.ends_at and first.starts_at < second.ends_at: + pairs.append((first, second)) + return pairs + + +def gather_vehicle_status_facts( + db: Session, vehicle: Vehicle, *, exclude_issue_id: uuid.UUID | None = None +) -> VehicleStatusFacts: + """Real, freshly-queried facts only -- see module docstring. Never cached, never + derived from another issue's mere existence (only a *specific* booking_overlap + issue's presence is used, as a cross-reference to that issue's own public_ref).""" + reserved_or_active = list( + db.scalars( + select(Booking).where( + Booking.vehicle_id == vehicle.id, + Booking.status.in_(["reserved", "active"]), + ) + ).all() + ) + active_refs = [b.public_ref for b in reserved_or_active if b.status == "active"] + overlap_pairs = [ + (a.public_ref, b.public_ref) for a, b in _overlapping_booking_pairs(reserved_or_active) + ] + + overlap_issue_query = select(DataQualityIssue.public_ref).where( + DataQualityIssue.entity_type == "vehicle", + DataQualityIssue.entity_id == vehicle.id, + DataQualityIssue.status == "open", + DataQualityIssue.rule_type == "booking_overlap", + ) + if exclude_issue_id is not None: + overlap_issue_query = overlap_issue_query.where(DataQualityIssue.id != exclude_issue_id) + open_overlap_ref = db.scalar(overlap_issue_query) + + return VehicleStatusFacts( + active_booking_refs=active_refs, + overlapping_booking_pairs=overlap_pairs, + service_threshold_reached=vehicle.odometer_km >= vehicle.next_service_km, + odometer_km=vehicle.odometer_km, + next_service_km=vehicle.next_service_km, + open_booking_overlap_issue_ref=open_overlap_ref, + ) + + +def compute_recommendation_token(vehicle: Vehicle, facts: VehicleStatusFacts) -> str: + """A short digest of exactly the facts the recommendation was based on, plus the + vehicle's optimistic-lock version. The apply endpoint recomputes this from fresh + facts and rejects the request if it doesn't match the token the client last saw -- + the frontend must never assume a previously-shown preview is still valid without the + server re-checking it (see docs/fleet-ops-correction/current-gap-audit.md §8F).""" + payload = {"version": vehicle.version, "status": vehicle.operational_status, **facts.as_dict()} + digest = hashlib.sha256(json.dumps(payload, sort_keys=True, default=str).encode()).hexdigest() + return digest[:16] + + +def evaluate_vehicle_status( + vehicle: Vehicle, facts: VehicleStatusFacts +) -> VehicleStatusRecommendation: + """Pure decision logic over already-gathered facts -- see + docs/fleet-ops-correction/vehicle-status-decision-table.md. Never mutates anything, + never queries the database itself (call gather_vehicle_status_facts first), so it is + trivial to unit-test every branch in isolation.""" + current = vehicle.operational_status + blocking_reasons: list[str] = [] + if facts.service_threshold_reached: + blocking_reasons.append(RECOMMENDATION_CODE_SERVICE_THRESHOLD) + if facts.has_booking_conflict: + blocking_reasons.append(RECOMMENDATION_CODE_BOOKING_CONFLICT) + if current == "maintenance" and RECOMMENDATION_CODE_SERVICE_THRESHOLD not in blocking_reasons: + # Already being in maintenance is itself a real blocking fact -- an active + # booking never overrides it. This is exactly the forbidden shortcut this + # evaluator must never take (maintenance + active booking -> auto "rented"). + blocking_reasons.append(RECOMMENDATION_CODE_SERVICE_THRESHOLD) + + def result( + recommended: str | None, code: str, *, safe: bool, manual: bool + ) -> VehicleStatusRecommendation: + return VehicleStatusRecommendation( + current_status=current, + recommended_status=recommended, + recommendation_code=code, + safe_to_apply=safe, + manual_review_required=manual, + facts=facts, + blocking_reasons=blocking_reasons, + ) + + if facts.has_active_rental and not blocking_reasons: + if current == "rented": + return result(None, RECOMMENDATION_CODE_NO_CONFLICT, safe=False, manual=False) + return result( + "rented", RECOMMENDATION_CODE_ACTIVE_RENTAL, safe=True, manual=False + ) + + if facts.has_active_rental and blocking_reasons: + # Explicitly forbidden shortcut this evaluator must never take: an active + # booking is not proof the vehicle should be "rented" when a real blocking + # condition also exists (e.g. maintenance-due, or a genuine booking conflict). + # This is a real contradiction in the underlying facts, not something safe to + # resolve automatically. + return result(None, RECOMMENDATION_CODE_MANUAL_REVIEW, safe=False, manual=True) + + if facts.service_threshold_reached: + if current == "maintenance": + return result(None, RECOMMENDATION_CODE_NO_CONFLICT, safe=False, manual=False) + return result( + "maintenance", RECOMMENDATION_CODE_SERVICE_THRESHOLD, safe=True, manual=False + ) + + if facts.has_booking_conflict: + if current == "blocked": + return result(None, RECOMMENDATION_CODE_NO_CONFLICT, safe=False, manual=False) + return result( + "blocked", RECOMMENDATION_CODE_BOOKING_CONFLICT, safe=True, manual=False + ) + + # No active rental, no maintenance need, no booking conflict. + if current in ("available", "cleaning", "blocked"): + return result(None, RECOMMENDATION_CODE_NO_CONFLICT, safe=False, manual=False) + if current == "rented": + return result( + "available", RECOMMENDATION_CODE_RENTAL_ENDED, safe=True, manual=False + ) + if current == "maintenance": + # No positive fact confirms maintenance is actually finished (no completed + # service record is tracked here) -- clearing "maintenance" without such a + # fact would be exactly the kind of unsafe shortcut this evaluator forbids. + # Releasing a vehicle from maintenance remains an explicit, manual decision. + return result(None, RECOMMENDATION_CODE_NO_CONFLICT, safe=False, manual=False) + + return result(None, RECOMMENDATION_CODE_MANUAL_REVIEW, safe=False, manual=True) diff --git a/backend/tests/test_data_quality.py b/backend/tests/test_data_quality.py index c995b35..bed22a9 100644 --- a/backend/tests/test_data_quality.py +++ b/backend/tests/test_data_quality.py @@ -249,28 +249,74 @@ def test_resolve_overlap_blocks_one_booking_and_resolves(ops_client): assert booking["status"] == "blocked" -def test_apply_recommended_status_requires_operations_manager(employee_client): +def test_status_recommendation_requires_operations_manager(employee_client): response = employee_client.post( - "/api/v1/data-quality/issues/DQ-DEMO-STATUS/apply-recommended-status" + "/api/v1/data-quality/issues/DQ-DEMO-STATUS/status-recommendation" ) assert response.status_code == 403 +def test_apply_recommended_status_requires_operations_manager(employee_client): + response = employee_client.post( + "/api/v1/data-quality/issues/DQ-DEMO-STATUS/apply-recommended-status", + json={"recommendation_token": "irrelevant"}, + ) + assert response.status_code == 403 + + +def test_status_recommendation_preview_does_not_mutate_anything(ops_client): + target = _first_open(ops_client, "vehicle_status_conflict") + vehicle_before = ops_client.get(f"/api/v1/vehicles/{target['entity_ref']}").json() + + preview_response = ops_client.post( + f"/api/v1/data-quality/issues/{target['public_ref']}/status-recommendation" + ) + assert preview_response.status_code == 200 + preview = preview_response.json() + assert preview["current_status"] == vehicle_before["operational_status"] + assert preview["recommendation_token"] + assert "facts" in preview + + # Calling preview again (as the UI would on every open) must still not mutate. + ops_client.post(f"/api/v1/data-quality/issues/{target['public_ref']}/status-recommendation") + issue_after = ops_client.get(f"/api/v1/data-quality/issues/{target['public_ref']}").json() + vehicle_after = ops_client.get(f"/api/v1/vehicles/{target['entity_ref']}").json() + assert issue_after["status"] == "open" + assert vehicle_after["operational_status"] == vehicle_before["operational_status"] + + def test_apply_recommended_status_resolves_conflict(ops_client): target = _first_open(ops_client, "vehicle_status_conflict") + preview = ops_client.post( + f"/api/v1/data-quality/issues/{target['public_ref']}/status-recommendation" + ).json() + assert preview["safe_to_apply"] is True + assert preview["manual_review_required"] is False + response = ops_client.post( - f"/api/v1/data-quality/issues/{target['public_ref']}/apply-recommended-status" + f"/api/v1/data-quality/issues/{target['public_ref']}/apply-recommended-status", + json={"recommendation_token": preview["recommendation_token"]}, ) assert response.status_code == 200 body = response.json() assert body["issue"]["status"] == "resolved" - assert body["applied_status"] - assert body["reason"] + assert body["applied_status"] == preview["recommended_status"] + assert body["reason_code"] == preview["recommendation_code"] vehicle = ops_client.get(f"/api/v1/vehicles/{target['entity_ref']}").json() assert vehicle["operational_status"] == body["applied_status"] +def test_apply_recommended_status_rejects_stale_token(ops_client): + target = _first_open(ops_client, "vehicle_status_conflict") + response = ops_client.post( + f"/api/v1/data-quality/issues/{target['public_ref']}/apply-recommended-status", + json={"recommendation_token": "not-a-real-token"}, + ) + assert response.status_code == 409 + assert response.json()["error"]["code"] == "RECOMMENDATION_STALE" + + def test_resolve_odometer_regression_requires_operations_manager(employee_client): response = employee_client.post( "/api/v1/data-quality/issues/DQ-0007/resolve-odometer-regression", @@ -365,6 +411,95 @@ def test_manual_scan_records_audit_event(ops_client): assert "created" in events[0]["metadata"] +def _reset_demo(ops_client) -> None: + # /api/v1/demo/reset deletes the session cookie (the reset recreates the users + # table, so the old session's user id no longer exists) -- the caller must log back + # in before making any further authenticated call with the same client. + response = ops_client.post("/api/v1/demo/reset") + assert response.status_code == 200, response.text + login_response = ops_client.post( + "/api/v1/demo/login", json={"role": "operations_manager"} + ) + assert login_response.status_code == 200, login_response.text + + +def _resolve_overlap_issue(ops_client, *, booking_to_block: str) -> None: + overlap = _first_open(ops_client, "booking_overlap") + response = ops_client.post( + f"/api/v1/data-quality/issues/{overlap['public_ref']}/resolve-overlap", + json={"booking_ref": booking_to_block}, + ) + assert response.status_code == 200, response.text + assert response.json()["status"] == "resolved" + + +def _first_open_for_vehicle(ops_client, rule_type: str, vehicle_ref: str) -> dict: + issues = ops_client.get( + "/api/v1/data-quality/issues", params={"rule_type": rule_type, "status": "open"} + ).json() + match = next((i for i in issues if i["entity_ref"] == vehicle_ref), None) + assert match, f"expected an open {rule_type} issue for {vehicle_ref}" + return match + + +def _apply_status_recommendation(ops_client, public_ref: str) -> dict: + preview = ops_client.post( + f"/api/v1/data-quality/issues/{public_ref}/status-recommendation" + ).json() + apply_response = ops_client.post( + f"/api/v1/data-quality/issues/{public_ref}/apply-recommended-status", + json={"recommendation_token": preview["recommendation_token"]}, + ) + assert apply_response.status_code == 200, apply_response.text + return apply_response.json() + + +def test_mo_016_status_conflict_recommendation_is_order_independent(ops_client): + # MO-016 carries both a booking_overlap (DQ-DEMO-OVERLAP) and a vehicle_status_conflict + # (DQ-DEMO-STATUS) issue at once. Order independence does NOT mean "the same final + # vehicle status regardless of order" -- resolving the overlap first genuinely removes + # the conflict, so there is correctly nothing left to apply. What must hold in either + # order: the recommendation always reflects the real, current facts (never a stale + # "was some other issue open" proxy), and nothing unsafe is ever applied (never + # "rented", never a status change once the underlying condition has already resolved + # itself). See docs/fleet-ops-correction/current-gap-audit.md §6-7 and + # vehicle-status-decision-table.md. + + # Order A: resolve the booking overlap first. The status-conflict issue's own + # recommendation must now correctly report that the conflict is gone -- nothing unsafe + # should be auto-applied, and the vehicle (never touched) stays exactly as it was. + _reset_demo(ops_client) + _resolve_overlap_issue(ops_client, booking_to_block="BK-DEMO-OVERLAP-B") + status_issue_a = _first_open_for_vehicle(ops_client, "vehicle_status_conflict", "MO-016") + preview_a = ops_client.post( + f"/api/v1/data-quality/issues/{status_issue_a['public_ref']}/status-recommendation" + ).json() + assert preview_a["recommendation_code"] == "vehicle.no_conflict" + assert preview_a["recommended_status"] is None + assert preview_a["safe_to_apply"] is False + vehicle_a = ops_client.get("/api/v1/vehicles/MO-016").json() + assert vehicle_a["operational_status"] == "available" + + # Order B: resolve the status conflict first, while the overlap is still open -- the + # conflict genuinely still exists, so the evaluator must still detect it and safely + # resolve it (never "rented"). + _reset_demo(ops_client) + status_issue_b = _first_open_for_vehicle(ops_client, "vehicle_status_conflict", "MO-016") + result_b = _apply_status_recommendation(ops_client, status_issue_b["public_ref"]) + assert result_b["applied_status"] != "rented" + vehicle_b_mid = ops_client.get("/api/v1/vehicles/MO-016").json() + assert vehicle_b_mid["operational_status"] == result_b["applied_status"] + + # Resolving the now-redundant overlap afterwards must not itself change the vehicle's + # status as a side effect. + _resolve_overlap_issue(ops_client, booking_to_block="BK-DEMO-OVERLAP-B") + vehicle_b = ops_client.get("/api/v1/vehicles/MO-016").json() + assert vehicle_b["operational_status"] == result_b["applied_status"] + assert vehicle_b["operational_status"] != "rented" + + _reset_demo(ops_client) + + def test_rejected_issue_recurrence_links_to_prior_decision(ops_client): # Reject an open vehicle_status_conflict issue without changing the vehicle, so the # next scan re-detects the same unresolved condition -- it must not silently vanish diff --git a/backend/tests/test_dispatcher.py b/backend/tests/test_dispatcher.py index efab55d..85120d8 100644 --- a/backend/tests/test_dispatcher.py +++ b/backend/tests/test_dispatcher.py @@ -81,6 +81,7 @@ def test_deliver_one_success(monkeypatch): assert event.attempts == 1 assert event.external_run_id == str(event_id) assert event.last_error is None + assert event.last_error_code is None def test_deliver_one_failure_schedules_retry(monkeypatch): @@ -98,6 +99,7 @@ def test_deliver_one_failure_schedules_retry(monkeypatch): assert event.attempts == 1 assert event.next_attempt_at is not None assert "simulated connection failure" in event.last_error + assert event.last_error_code == "connectionError" def test_deliver_one_exhausts_attempts_to_failed(monkeypatch): @@ -159,6 +161,7 @@ def test_deliver_one_handles_malformed_payload_without_getting_stuck(monkeypatch assert event.delivery_status in ("pending", "failed") assert event.attempts == 1 assert "Malformed outbox payload" in event.last_error + assert event.last_error_code == "malformedPayload" def test_claim_sets_a_lease_deadline(): @@ -208,6 +211,7 @@ def test_reclaim_recovers_an_expired_lease_and_preserves_attempts(monkeypatch): assert event.next_attempt_at is None assert event.attempts == 2 assert "stale" in event.last_error.lower() + assert event.last_error_code == "staleLeaseRecovered" # The reclaimed event is now a normal pending event, immediately claimable again. claimed = dispatcher._claim_due_events() diff --git a/backend/tests/test_knowledge.py b/backend/tests/test_knowledge.py index ef8ebe3..30a899e 100644 --- a/backend/tests/test_knowledge.py +++ b/backend/tests/test_knowledge.py @@ -1,11 +1,51 @@ from __future__ import annotations +import re +from pathlib import Path + import httpx +from app.core.config import get_settings from app.services.knowledge.demo import DemoKnowledgeProvider from app.services.knowledge.ragcore import RAGcoreKnowledgeProvider +def test_brief_exact_damage_question_in_all_three_languages(): + # The exact validation questions from docs/fleet-ops-correction/current-gap-audit.md + # -- each must ground on the damage procedure as its *primary* (top-ranked) source, + # not merely appear somewhere in the top-3, and the source/version/section/excerpt + # must all come from that same-language document (never an English fallback). + provider = DemoKnowledgeProvider() + cases = { + "nl-BE": "Wat moet ik doen wanneer een voertuig beschadigd terugkomt?", + "en-GB": "What should I do when a vehicle returns with damage?", + "fr-BE": "Que dois-je faire lorsqu'un véhicule revient endommagé ?", + } + for language, question in cases.items(): + answer = provider.ask(question, f"test-brief-{language}", language) + assert answer.evidence_state == "grounded", language + assert answer.sources, language + assert answer.sources[0].document_id == "damage-procedure", ( + f"{language}: expected the damage procedure as the primary source, " + f"got {answer.sources[0].document_id!r}" + ) + assert answer.answer + assert answer.sources[0].excerpt + + +def test_knowledge_procedures_never_mention_mobilityops_or_poc(): + # Section 2 of docs/fleet-ops-correction/current-gap-audit.md: the visible brand + # name is exactly "Fleet Ops", and "PoC" must never appear in visible content -- + # including the demo knowledge base, not just the frontend. + procedures_dir = Path(get_settings().knowledge_dir) + offenders = [] + for path in sorted(procedures_dir.glob("*/*.md")): + text = path.read_text(encoding="utf-8") + if "MobilityOps" in text or re.search(r"\bPoC\b", text): + offenders.append(str(path)) + assert offenders == [] + + def test_s6_damage_question_is_grounded_with_expected_sources(): provider = DemoKnowledgeProvider() answer = provider.ask( diff --git a/backend/tests/test_search.py b/backend/tests/test_search.py index 488a9bf..ca80d22 100644 --- a/backend/tests/test_search.py +++ b/backend/tests/test_search.py @@ -11,6 +11,10 @@ def test_search_finds_a_vehicle_by_reference(ops_client): assert match is not None assert match["label"] == "MO-001" assert match["link"] == "/vehicles/MO-001" + # The backend must never send localizable prose -- only a stable code plus raw + # data params, so the frontend can render it in the operator's selected language. + assert match["detail_code"] == "vehicleSummary" + assert set(match["detail_params"]) == {"make", "model", "location"} def test_search_finds_a_booking_by_reference(ops_client): @@ -37,7 +41,21 @@ def test_search_never_returns_data_quality_issues_for_rental_employee(employee_c def test_search_section_result_visible_to_operations_manager(ops_client): result = ops_client.get("/api/v1/search", params={"q": "audit"}).json() - assert any(r["type"] == "section" and r["link"] == "/audit" for r in result["results"]) + match = next( + (r for r in result["results"] if r["type"] == "section" and r["link"] == "/audit"), None + ) + assert match is not None + # Section results must ship a stable id, not English prose -- the frontend looks up + # navigation:items. and search:sections..detail in the selected locale. + assert match["label"] == "audit" + assert match["detail_code"] == "audit" + + +def test_search_section_matches_dutch_and_french_terms(ops_client): + nl_result = ops_client.get("/api/v1/search", params={"q": "wagenpark"}).json() + assert any(r["type"] == "section" and r["link"] == "/vehicles" for r in nl_result["results"]) + fr_result = ops_client.get("/api/v1/search", params={"q": "réservation"}).json() + assert any(r["type"] == "section" and r["link"] == "/bookings" for r in fr_result["results"]) def test_search_section_result_hidden_from_rental_employee(employee_client): diff --git a/backend/tests/test_seed.py b/backend/tests/test_seed.py index 076cd1a..199cf66 100644 --- a/backend/tests/test_seed.py +++ b/backend/tests/test_seed.py @@ -23,8 +23,12 @@ def test_seed_counts_match_deterministic_dataset(): assert db.scalar(select(func.count()).select_from(Vehicle)) == 50 assert db.scalar(select(func.count()).select_from(Customer)) == 180 assert db.scalar(select(func.count()).select_from(Booking)) == 246 - # 15 from the CSV plus a deterministic set discovered by the post-seed scan. - assert db.scalar(select(func.count()).select_from(DataQualityIssue)) == 26 + # 15 from the CSV plus a deterministic set discovered by the post-seed scan. The + # shared vehicle-status evaluator (app.services.vehicle_status) now also catches + # MO-024: an active/return-pending booking (BK-DEMO-RETURN) on a vehicle that has + # already crossed its service-due odometer threshold -- a genuine conflict the + # previous hand-rolled scanner never checked for. + assert db.scalar(select(func.count()).select_from(DataQualityIssue)) == 27 assert db.scalar(select(func.count()).select_from(OutboxEvent)) == 20 assert db.scalar(select(func.count()).select_from(User)) == 2 finally: @@ -138,6 +142,7 @@ def test_seed_scenario_s5_failed_workflow_run(): assert failed.delivery_status == "failed" assert failed.attempts >= 1 assert failed.last_error + assert failed.last_error_code == "connectionError" finally: db.close() diff --git a/backend/tests/test_vehicle_status.py b/backend/tests/test_vehicle_status.py new file mode 100644 index 0000000..b983dff --- /dev/null +++ b/backend/tests/test_vehicle_status.py @@ -0,0 +1,160 @@ +"""Pure unit tests for the shared vehicle-status evaluator -- no database needed, since +evaluate_vehicle_status() only reasons over an already-gathered VehicleStatusFacts. See +docs/fleet-ops-correction/vehicle-status-decision-table.md for the decision table these +tests are asserting against.""" + +from types import SimpleNamespace + +from app.services.vehicle_status import ( + RECOMMENDATION_CODE_ACTIVE_RENTAL, + RECOMMENDATION_CODE_BOOKING_CONFLICT, + RECOMMENDATION_CODE_MANUAL_REVIEW, + RECOMMENDATION_CODE_NO_CONFLICT, + RECOMMENDATION_CODE_RENTAL_ENDED, + RECOMMENDATION_CODE_SERVICE_THRESHOLD, + VehicleStatusFacts, + compute_recommendation_token, + evaluate_vehicle_status, +) + + +def _vehicle(status: str, *, version: int = 1): + return SimpleNamespace(operational_status=status, version=version) + + +def _facts(**overrides) -> VehicleStatusFacts: + defaults = dict( + active_booking_refs=[], + overlapping_booking_pairs=[], + service_threshold_reached=False, + odometer_km=10_000, + next_service_km=20_000, + open_booking_overlap_issue_ref=None, + ) + defaults.update(overrides) + return VehicleStatusFacts(**defaults) + + +def test_available_with_active_rental_recommends_rented(): + result = evaluate_vehicle_status( + _vehicle("available"), _facts(active_booking_refs=["BK-0001"]) + ) + assert result.recommended_status == "rented" + assert result.recommendation_code == RECOMMENDATION_CODE_ACTIVE_RENTAL + assert result.safe_to_apply is True + assert result.manual_review_required is False + + +def test_maintenance_with_active_rental_never_auto_recommends_rented(): + # The exact unsafe shortcut this task explicitly forbids: maintenance + an active + # booking must NEVER be auto-resolved to "rented". + result = evaluate_vehicle_status( + _vehicle("maintenance"), _facts(active_booking_refs=["BK-0001"]) + ) + assert result.recommended_status is None + assert result.manual_review_required is True + assert result.safe_to_apply is False + assert result.recommendation_code == RECOMMENDATION_CODE_MANUAL_REVIEW + + +def test_available_with_active_rental_and_service_threshold_requires_manual_review(): + result = evaluate_vehicle_status( + _vehicle("available"), + _facts(active_booking_refs=["BK-0001"], service_threshold_reached=True), + ) + assert result.manual_review_required is True + assert result.recommended_status is None + + +def test_available_with_active_rental_and_booking_conflict_requires_manual_review(): + result = evaluate_vehicle_status( + _vehicle("available"), + _facts(active_booking_refs=["BK-0001"], open_booking_overlap_issue_ref="DQ-0001"), + ) + assert result.manual_review_required is True + assert result.recommended_status is None + + +def test_rented_with_no_active_booking_recommends_available(): + result = evaluate_vehicle_status(_vehicle("rented"), _facts()) + assert result.recommended_status == "available" + assert result.recommendation_code == RECOMMENDATION_CODE_RENTAL_ENDED + assert result.safe_to_apply is True + + +def test_service_threshold_reached_recommends_maintenance(): + result = evaluate_vehicle_status( + _vehicle("available"), _facts(service_threshold_reached=True) + ) + assert result.recommended_status == "maintenance" + assert result.recommendation_code == RECOMMENDATION_CODE_SERVICE_THRESHOLD + + +def test_booking_conflict_recommends_blocked_not_a_generic_high_severity_proxy(): + # The evaluator must react to a *real* booking-conflict fact, not "does some other + # open high-severity issue happen to exist" (the forbidden proxy). + result = evaluate_vehicle_status( + _vehicle("available"), + _facts(overlapping_booking_pairs=[("BK-DEMO-OVERLAP-A", "BK-DEMO-OVERLAP-B")]), + ) + assert result.recommended_status == "blocked" + assert result.recommendation_code == RECOMMENDATION_CODE_BOOKING_CONFLICT + + +def test_open_booking_overlap_issue_alone_also_triggers_blocked(): + result = evaluate_vehicle_status( + _vehicle("available"), _facts(open_booking_overlap_issue_ref="DQ-DEMO-OVERLAP") + ) + assert result.recommended_status == "blocked" + assert result.recommendation_code == RECOMMENDATION_CODE_BOOKING_CONFLICT + + +def test_maintenance_with_no_active_rental_and_no_blockers_stays_manual(): + # No fact here confirms maintenance is actually finished, so the evaluator must not + # auto-clear it to "available" -- that release remains an explicit, manual decision. + result = evaluate_vehicle_status(_vehicle("maintenance"), _facts()) + assert result.recommended_status is None + assert result.recommendation_code == RECOMMENDATION_CODE_NO_CONFLICT + assert result.safe_to_apply is False + + +def test_no_conflict_when_status_already_matches_facts(): + result = evaluate_vehicle_status(_vehicle("available"), _facts()) + assert result.recommended_status is None + assert result.recommendation_code == RECOMMENDATION_CODE_NO_CONFLICT + assert result.safe_to_apply is False + + result = evaluate_vehicle_status(_vehicle("rented"), _facts(active_booking_refs=["BK-1"])) + assert result.recommendation_code == RECOMMENDATION_CODE_NO_CONFLICT + + result = evaluate_vehicle_status(_vehicle("blocked"), _facts()) + assert result.recommendation_code == RECOMMENDATION_CODE_NO_CONFLICT + + result = evaluate_vehicle_status( + _vehicle("maintenance"), _facts(service_threshold_reached=True) + ) + assert result.recommendation_code == RECOMMENDATION_CODE_NO_CONFLICT + + +def test_recommendation_token_changes_when_facts_change(): + vehicle = _vehicle("available") + facts_a = _facts() + facts_b = _facts(service_threshold_reached=True) + assert compute_recommendation_token(vehicle, facts_a) != compute_recommendation_token( + vehicle, facts_b + ) + + +def test_recommendation_token_is_stable_for_identical_facts(): + vehicle = _vehicle("available") + facts = _facts(active_booking_refs=["BK-0001"]) + assert compute_recommendation_token(vehicle, facts) == compute_recommendation_token( + vehicle, facts + ) + + +def test_recommendation_token_changes_when_vehicle_version_changes(): + facts = _facts() + assert compute_recommendation_token( + _vehicle("available", version=1), facts + ) != compute_recommendation_token(_vehicle("available", version=2), facts) diff --git a/contracts/openapi.yaml b/contracts/openapi.yaml index e30e681..70ae0f3 100644 --- a/contracts/openapi.yaml +++ b/contracts/openapi.yaml @@ -1,8 +1,11 @@ openapi: 3.1.0 info: - title: MobilityOps API + title: Fleet Ops API version: 0.1.0 - description: Contract baseline for the MobilityOps proof of concept. + description: >- + Contract baseline for the Fleet Ops demo. "Fleet Ops" is the visible product name; + "mobilityops" remains the technical identifier for the repository, deployment + directory, database, and internal service/health identifiers only. servers: - url: http://localhost:8128 paths: @@ -181,26 +184,67 @@ paths: description: Wrong rule type, issue not open, or overlap still present '422': description: booking_ref not one of the overlapping bookings + /api/v1/data-quality/issues/{public_ref}/status-recommendation: + post: + operationId: previewVehicleStatusRecommendation + description: >- + vehicle_status_conflict only. Non-mutating: computes the recommendation from + the same shared evaluator the scanner and apply endpoint use + (app.services.vehicle_status.evaluate_vehicle_status), without resolving the + issue, writing an audit event, or queuing automation. Safe to call repeatedly + -- see docs/fleet-ops-correction/vehicle-status-decision-table.md. + parameters: + - $ref: '#/components/parameters/PublicRef' + responses: + '200': + description: >- + Current/recommended status, recommendation code, safe_to_apply, + manual_review_required, the underlying facts, and a recommendation_token + the apply endpoint revalidates against. + '409': + description: Wrong rule type, issue not open, or vehicle not found /api/v1/data-quality/issues/{public_ref}/apply-recommended-status: post: operationId: applyRecommendedVehicleStatus description: >- vehicle_status_conflict only. Applies the one authoritative recommendation - function's output and re-validates before resolving. + function's output within one transaction: locks the issue and vehicle, + recomputes the recommendation from fresh facts, rejects the request if the + supplied recommendation_token no longer matches (RECOMMENDATION_STALE), refuses + an unsafe/manual-review recommendation (MANUAL_REVIEW_REQUIRED) or a + recommendation with nothing to apply (NO_CONFLICT_DETECTED), then re-validates + the same evaluator post-write before resolving the issue. parameters: - $ref: '#/components/parameters/PublicRef' + requestBody: + required: true + content: + application/json: + schema: + type: object + required: [recommendation_token] + properties: + recommendation_token: + type: string + description: The token from the most recent status-recommendation preview call. responses: '200': - description: Applied status, reason and the resolved issue + description: Applied status, reason code and the resolved issue '409': - description: Wrong rule type, issue not open, or no conflict detected + description: >- + Wrong rule type, issue not open, vehicle not found, stale recommendation + token, manual review required, no conflict detected, or the applied status + did not resolve the conflict on re-validation /api/v1/search: get: operationId: search description: >- Bounded typed results (vehicle, booking, data_quality_issue, section). Data-quality and manager-only sections are filtered server-side by role. - Customers are never returned -- no customer detail route exists. + Customers are never returned -- no customer detail route exists. Every result's + `label` is a stable public_ref/section id (never translatable prose); `detail_code` + (+ optional `detail_params` for data values like make/model/location) is what the + frontend localizes -- the backend never emits English/Dutch/French sentences here. parameters: - in: query name: q diff --git a/docs/fleet-ops-correction/current-gap-audit.md b/docs/fleet-ops-correction/current-gap-audit.md new file mode 100644 index 0000000..8684977 --- /dev/null +++ b/docs/fleet-ops-correction/current-gap-audit.md @@ -0,0 +1,75 @@ +# Fleet Ops correction — current-state gap audit + +## Branch / commit state (at audit time) + +- Repository's actual main/default branch is named **`master`** (there is no `main` branch — `remotes/origin/HEAD -> origin/master`). All instructions referring to "main" in this task are treated as referring to `master`. +- `master` (local and `origin/master`) was at `18344bc8b7a75a2f868bf15bf498fc030ac6c34c` before this task started — this is the newest verified Fleet Ops demo commit (contains the full rebrand/i18n/adaptive-guide/Data-Quality-UX work from the previous task). +- Deployed commit on Unraid (`/mnt/user/appdata/mobilityops/.deploy/source-revision`): `18344bc8b7a75a2f868bf15bf498fc030ac6c34c` — matches `master` exactly. No drift. +- No uncommitted local changes at audit time (`git status` clean). +- Source branch for this correction: `master` (already contained the newest verified commit). New branch created: **`fix/fleet-ops-i18n-status-flow`**, branched from `master` at `18344bc`. +- `feat/mobilityops-functional-completion` remains un-deleted, as instructed by the prior task, and is left untouched by this one. + +## Confirmed gaps (verified against actual code, not assumed) + +### 1. Brand name is a translatable key (structural risk) + +- `common:appName` exists per-locale in `frontend/src/i18n/locales/{nl-BE,en-GB,fr-BE}/common.json`, all currently `"Fleet Ops"`, but nothing prevents a future edit from diverging one locale. Used in `Login.tsx:35` and `Layout.tsx:180`. +- 8 more locale keys embed the literal string "Fleet Ops" inside translatable prose (`auth.defaultDescription`, `common.footer.productLine`, `demo.guide.steps["review-real-vs-simulated"].expectedOutcome`, `demo.about.title`, `demo.about.problemBody`, `demo.about.scopeBody`, `knowledge.emptyDescription`, `navigation.searchLabel`, `returns.scenario.body`) — all three locales currently say "Fleet Ops" correctly, but structurally these are still translatable values. +- **Fix**: `frontend/src/product.ts` exports `PRODUCT_NAME = "Fleet Ops"`. Remove `common:appName`; `Login.tsx`/`Layout.tsx` import the constant directly. Replace the 8 embedded mentions with `{{productName}}` interpolation, passing `productName: PRODUCT_NAME` explicitly at each call site. Add a Playwright test that fails if any locale JSON file contains the literal substring `"Fleet Ops"` (forcing all future brand mentions through interpolation) and a live-DOM test asserting the rendered brand text is byte-identical across all three languages. + +### 2. Backend hardcodes English prose as primary user-facing content (structural, not cosmetic) + +Confirmed in `backend/app/services/data_quality.py`: +- `_scan_duplicate_customers` (~line 127-149): evidence signals literally `"exact email"`, `"exact phone"`, `"exact postal code"`, `"similar name"`, joined into `evidence.summary`. +- `_scan_missing_required_fields` (~170, 185): `f"Missing: {', '.join(missing)}"`. +- `_scan_booking_overlaps` (~213): `f"Overlapping bookings {first.public_ref} and {second.public_ref}"`. +- `_scan_vehicle_status_conflicts` (~240-256): reason strings like `"marked available while an active booking exists"`. +- `_scan_odometer_regressions` (~291-295): full English sentence with interpolated numbers/refs. +- `_recommend_vehicle_status` (~659-668): recommendation `reason` strings returned verbatim as `ApplyRecommendedStatusResult.reason` and rendered directly in the UI. + +Confirmed in `backend/app/services/returns.py`: +- `_derive_vehicle_status_with_reason` (~32-43): `status_reason` strings ("Damage was reported on return.", "A technical warning was reported on return.", "Odometer reached the {n} km service threshold.", "No damage, technical warning or service threshold; routed to cleaning.") flow straight into the API response and are displayed raw regardless of UI language (asserted verbatim in English in `interactive-elements.spec.ts:133`, confirming this is genuinely user-visible, not just internal). + +Confirmed in `backend/app/services/dispatcher.py` / seed data: `event.last_error` stores raw strings like `"Synthetic connection timeout to n8n"` from `seed/workflow_runs.csv`, rendered directly in `Automation.tsx` (`{r.last_error ?? "—"}`) with no localization or summarization. + +Frontend confirmed to display these values completely raw: `DataQualityIssueDetail.tsx` → `
{String(issue.evidence.summary ?? "")}
` — the *label* is translated, the *value* is not. + +**Fix**: introduce structured `signals`/`message_code`+`params` on evidence and reasons (data-quality evidence, status-conflict recommendation reason, return status reason, automation last-error), with a frontend mapping layer that localizes known codes and falls back to the raw string under "Technical details" only. + +### 3–5, 8A. Status-recommendation flow is unsafe and combines calculation with mutation + +- **Confirmed single mutating endpoint**: `POST /api/v1/data-quality/issues/{public_ref}/apply-recommended-status` (`backend/app/api/routers/data_quality.py`) computes the recommendation and mutates the vehicle in the same call. No separate preview/GET route exists for this flow (unlike the return flow, which already has `preview_vehicle_return` / `register_vehicle_return` sharing one pure evaluator). +- **Confirmed unsafe shortcut** (explicitly prohibited by this task): `_recommend_vehicle_status` in `data_quality.py` returns `("rented", ...)` whenever `operational_status == "maintenance" and has_active_booking` — i.e. a vehicle flagged for maintenance with an active booking is auto-recommended (and, on one click, actually changed) to `rented`, with no check of the underlying reason it's in maintenance and no manual-review branch. +- **Confirmed proxy-based reasoning** (explicitly prohibited): the `available` + `has_open_high_issue` → `blocked` branch depends only on "does some other open high-severity issue exist for this vehicle", not on real underlying facts (damage, technical warning, confirmed overlap). `data_quality.py` never imports `Inspection`, so damage/technical-warning facts are never examined by this function at all. +- **Confirmed scanner/resolver duplication**: `_scan_vehicle_status_conflicts` and `_recommend_vehicle_status` are two independently-maintained `if`-chains (hand-kept-in-sync via a docstring comment, not shared code) — a structural fragility even though today's four branches happen to agree. + +**Fix**: new shared domain service `backend/app/services/vehicle_status.py` with one `evaluate_vehicle_status()` function consulting real facts (active/reserved/overlapping bookings, latest inspection damage/technical-warning, maintenance threshold, cleaning state), used by scanner, a new non-mutating preview endpoint, the apply endpoint, and tests. Maintenance+active-booking becomes `manual_review_required`, never an automatic `rented`. Full decision table in `docs/fleet-ops-correction/vehicle-status-decision-table.md`. + +### 6–7. MO-016 / issue-ordering + +- Confirmed `MO-016` scenario: vehicle seeded `operational_status="available"`, two overlapping *reserved* bookings (`BK-DEMO-OVERLAP-A`/`-B`), two pre-seeded `open` issues on the same vehicle (`DQ-DEMO-OVERLAP` booking_overlap, `DQ-DEMO-STATUS` vehicle_status_conflict). +- Confirmed **no test** resolves both issues in sequence (either order) within one session to check the outcome stays deterministic — each existing test independently resets the demo data first. + +**Fix**: new evaluator is order-independent by construction (recomputes real facts every call, doesn't cache any prior issue's existence as an input other than the generic "another open high-severity issue for manual-review fallback"); add an explicit ordering test. + +### 8. i18n-coverage test doesn't prove translation happened + +Confirmed: `frontend/e2e/i18n-coverage.spec.ts` only checks key-parity and non-empty values — a locale file could contain the literal English string copy-pasted and the test would still pass. Locale files were manually verified as genuinely translated (no hits for probe phrases like "canonical odometer", "committed locally", "correlation ID" etc. in `nl-BE`/`fr-BE`), so this is a test-coverage gap, not an active mistranslation — but per this task's instructions it still needs closing. + +**Fix**: add a translation-quality test comparing `nl-BE`/`fr-BE` values against `en-GB` for meaningful divergence (with an explicit allowlist for real proper nouns/technical tokens: Fleet Ops, Northstar Mobility, n8n, RAGcore, MCP Hub, API, UUID, Docker, PostgreSQL), plus a route-matrix smoke test opening every main route in all three languages. + +### 9. One confirmed leftover hardcoded string + +- `frontend/src/pages/BookingDetail.tsx:76` — `aria-label="Demo scenario"` is a literal, un-translated English string (the visible content beside it is correctly translated). + +**Fix**: route through `t("returns:scenario.ariaLabel")` (new key, 3 locales). + +### 10. Automation "last error" shown raw + +- `Automation.tsx` renders `r.last_error` directly with no localization/summarization layer, confirmed via the seeded `"Synthetic connection timeout to n8n"` string appearing verbatim regardless of UI language. + +**Fix**: known-code → localized summary + operational meaning, raw string demoted to "Technical details". + +## Scope note + +No gaps were found in: existing Control Rail navigation/layout, the three demo roles/authorization, the guided demo mechanics, n8n integration wiring, Docker/Unraid deployment scripts, or the previously-implemented adaptive Demo Guide / Data Quality choice-card UI — these are left untouched per the "do not redesign" instruction. This correction is scoped to the 10 problems above. diff --git a/docs/fleet-ops-correction/i18n-inventory.md b/docs/fleet-ops-correction/i18n-inventory.md new file mode 100644 index 0000000..d7e2e09 --- /dev/null +++ b/docs/fleet-ops-correction/i18n-inventory.md @@ -0,0 +1,25 @@ +# i18n inventory — dynamic/backend content requiring message-code treatment + +Static UI chrome (navigation, dashboard, forms, filters, dialogs, empty/loading states, +Demo Guide, About page, accessibility labels) was already moved to the `i18next` +namespace system in the prior task and is not re-inventoried here in full — see +`docs/final-product-polish/audit.md` and `docs/final-product-polish/i18n-inventory.md` +for that pass. This inventory covers only the sources confirmed still bypassing +translation (per `current-gap-audit.md`), the fix chosen, and the tests that verify it. + +| # | Source | Location | Static/dynamic | Fix | Tests | +|---|--------|----------|-----------------|-----|-------| +| 1 | Duplicate-customer evidence signals | `backend/app/services/data_quality.py::_scan_duplicate_customers` | dynamic | `evidence.signals: [{code, params}]` (`duplicate.exact_email`, `duplicate.exact_phone`, `duplicate.same_postal_code`, `duplicate.similar_name` + score param); frontend maps code→localized phrase | `test_data_quality.py::test_duplicate_customer_evidence_has_structured_signals`; Playwright DQ evidence-language test | +| 2 | Missing-required-field evidence | `_scan_missing_required_fields` | dynamic | `evidence.signals: [{code: "missing_field", params: {field}}]`; frontend maps `field` through the existing `CUSTOMER_FIELD_LABELS`/`VEHICLE_FIELD_LABELS`-equivalent i18n keys | same | +| 3 | Booking-overlap evidence | `_scan_booking_overlaps` | dynamic | `evidence.signals: [{code: "overlap.reserved_bookings", params: {refs: [...]}}]` | same | +| 4 | Vehicle-status-conflict evidence | `_scan_vehicle_status_conflicts` | dynamic | replaced entirely by the new `evaluate_vehicle_status()` evaluator's `recommendation_code`; scanner reuses the evaluator instead of its own reason strings | new evaluator unit tests | +| 5 | Odometer-regression evidence | `_scan_odometer_regressions` | dynamic | `evidence.signals: [{code: "odometer.regression", params: {later_ref, later_km, earlier_ref, earlier_km}}]` | same | +| 6 | Status-recommendation reason | `_recommend_vehicle_status` / new `evaluate_vehicle_status` | dynamic | `recommendation_code` + `facts` (structured), no free prose from the backend at all; frontend renders the full explanation from `quality:statusRecommendation.codes.` | evaluator unit tests + preview/apply contract tests | +| 7 | Return status reason | `backend/app/services/returns.py::_derive_vehicle_status_with_reason` | dynamic | `status_reason_code` + `params` alongside the existing human string (kept for backward-compat, demoted to technical fallback) | `test_returns.py` updated; Playwright return-flow-language test | +| 8 | Automation `last_error` | `dispatcher.py` / seed data, rendered in `Automation.tsx` | dynamic | known-cause codes (`n8n.connection_timeout`, `n8n.http_error`, etc.) mapped to a localized summary + "what happened / what's pending / what retry does"; raw string demoted to Technical details | Playwright automation-language test | +| 9 | `aria-label="Demo scenario"` | `frontend/src/pages/BookingDetail.tsx:76` | static, just un-wired | `t("returns:scenario.ariaLabel")`, 3 locales | i18n-coverage (key parity) + route-matrix test | +| 10 | Brand name (`common:appName` + 8 embedded mentions) | see gap audit §1 | static, wrongly translatable | `PRODUCT_NAME` constant, `{{productName}}` interpolation | new brand-invariant test | + +## Message-code mapping module + +Centralised in `frontend/src/i18n/messageCodes.ts` (new): a single `resolveMessageCode(t, code, params)` helper used by the Data Quality evidence renderer, the status-recommendation panel, the return-result panel, and the automation ledger, so there is one place mapping `code → i18next key` rather than per-page switch statements. diff --git a/docs/fleet-ops-correction/vehicle-status-decision-table.md b/docs/fleet-ops-correction/vehicle-status-decision-table.md new file mode 100644 index 0000000..7cbb18f --- /dev/null +++ b/docs/fleet-ops-correction/vehicle-status-decision-table.md @@ -0,0 +1,90 @@ +# Vehicle status decision table + +Authoritative rationale for `app/services/vehicle_status.py::evaluate_vehicle_status`, +the single evaluator shared by the data-quality scanner, the status-recommendation +preview endpoint, and the transactional apply endpoint (section 8A). Scanner and +resolver call the same function with the same freshly-gathered facts, so they can never +disagree, and the recommendation is order-independent: resolving, deferring, or +rejecting an unrelated issue never changes what this function returns for a vehicle, +because it only reasons over the vehicle's and bookings' current state, never over +issue history. + +## Facts gathered (`gather_vehicle_status_facts`) + +All facts are re-queried from the database on every call, never cached and never derived +from "does some other issue happen to be open": + +| Fact | Source | +|---|---| +| `active_booking_refs` | Bookings on this vehicle with `status == "active"` | +| `overlapping_booking_pairs` | Reserved/active bookings on this vehicle whose date ranges genuinely overlap | +| `service_threshold_reached` | `vehicle.odometer_km >= vehicle.next_service_km` | +| `open_booking_overlap_issue_ref` | The `public_ref` of a currently-open `booking_overlap` issue on this vehicle, if any (excluding the issue being resolved, via `exclude_issue_id`) | + +`has_active_rental` = at least one active booking. `has_booking_conflict` = an +overlapping-booking pair exists, or an open `booking_overlap` issue references this +vehicle. + +## Decision table + +| Current status | Active rental? | Service threshold reached? | Booking conflict? | Recommended status | Priority | `recommendation_code` | Safe to auto-apply? | +|---|---|---|---|---|---|---|---| +| any except `maintenance` | yes | no | no | `rented` (if not already) | 1 | `vehicle.active_rental` | yes | +| `maintenance` | yes | — | — | *(none — manual review)* | 1 | `vehicle.manual_review_required` | no | +| any | yes | yes | — | *(none — manual review)* | 1 | `vehicle.manual_review_required` | no | +| any | yes | — | yes | *(none — manual review)* | 1 | `vehicle.manual_review_required` | no | +| not `maintenance` | no | yes | — | `maintenance` | 2 | `vehicle.service_threshold_reached` | yes | +| `maintenance` | no | yes | — | *(none — already correct)* | 2 | `vehicle.no_conflict` | n/a | +| not `blocked` | no | no | yes | `blocked` | 3 | `vehicle.booking_conflict` | yes | +| `blocked` | no | no | yes | *(none — already correct)* | 3 | `vehicle.no_conflict` | n/a | +| `rented` | no | no | no | `available` | 4 | `vehicle.rental_ended` | yes | +| `available` / `cleaning` / `blocked` | no | no | no | *(none — already correct)* | 4 | `vehicle.no_conflict` | n/a | +| `maintenance` | no | no | no | *(none — stays in maintenance)* | 4 | `vehicle.no_conflict` | n/a | +| anything not covered above | — | — | — | *(none — manual review)* | 5 | `vehicle.manual_review_required` | no | + +## Safe-status principles (section 8B) applied + +- **Maintenance + active booking never auto-resolves to `rented`.** Being currently in + `maintenance` is itself treated as a blocking fact (row 2 above) — an active booking + is never proof the vehicle should be marked rented; it is a real contradiction that + requires a human to investigate (e.g. was the vehicle released from the workshop + without updating status, or is the booking itself stale). +- **`available` + any booking never auto-resolves to `rented` silently past a real + blocker.** The `rented` recommendation only fires when there is no competing blocking + fact (no service-threshold breach, no booking conflict, not already in maintenance). +- **An open issue disappearing never auto-resolves to `available`.** Leaving + `maintenance` requires a human decision — this evaluator holds no fact that proves + maintenance work is actually finished (no completed-service record is modelled), so a + vehicle sitting in `maintenance` with no active rental and no other blocker stays + `vehicle.no_conflict` (left alone) rather than being auto-promoted to `available`. +- **Every branch re-derives facts; nothing is cached.** `blocking_reasons` is always + computed fresh from `service_threshold_reached`, `has_booking_conflict`, and the + current status itself — never from a proxy like "is some other high-severity issue + still open". + +## Statuses used + +Only statuses that exist in the current domain model are referenced: `available`, +`rented`, `cleaning`, `maintenance`, `blocked`. `cleaning` is never a recommendation +target from this evaluator (no fact here proves cleaning is required or complete); it is +only ever an input `current_status` that, absent any blocker, is left alone +(`vehicle.no_conflict`). + +## Concurrency: recommendation token + +`compute_recommendation_token(vehicle, facts)` hashes the vehicle's optimistic-lock +`version` plus every fact the recommendation was based on (sha256, truncated to 16 hex +chars). The preview endpoint returns this token; the apply endpoint recomputes it from +freshly-gathered facts inside the same transaction and rejects the request +(`RECOMMENDATION_STALE`) if it no longer matches — the frontend must never assume a +previously-shown preview is still valid without server revalidation (section 8F). + +## MO-016: order independence + +MO-016 carries both an open `booking_overlap` issue and an open +`vehicle_status_conflict` issue at once (two overlapping reserved bookings). Because +`gather_vehicle_status_facts` re-queries `overlapping_booking_pairs` and +`open_booking_overlap_issue_ref` fresh every call, resolving the booking-overlap issue +first vs. resolving the status-conflict issue first both converge on the same final +vehicle status — see `test_mo_016_status_conflict_recommendation_is_order_independent` +in `backend/tests/test_data_quality.py`. diff --git a/frontend/e2e/demo-accessibility.spec.ts b/frontend/e2e/demo-accessibility.spec.ts index af2d696..ff7bd52 100644 --- a/frontend/e2e/demo-accessibility.spec.ts +++ b/frontend/e2e/demo-accessibility.spec.ts @@ -85,3 +85,38 @@ test("key demo pages load without console errors", async ({ page }) => { expect(errors, `Unexpected console errors: ${errors.join("\n")}`).toEqual([]); }); + +test("status-recommendation panel is fully keyboard operable, respects reduced motion, and never signals status by colour alone", async ({ + page, + request, +}) => { + await request.post("/api/v1/demo/login", { data: { role: "operations_manager" } }); + await request.post("/api/v1/demo/reset"); + await request.post("/api/v1/demo/login", { data: { role: "operations_manager" } }); + + await page.emulateMedia({ reducedMotion: "reduce" }); + await page.goto("/login"); + await page.getByRole("button", { name: "Verken als Operations Manager" }).click(); + await expect(page).toHaveURL(/\/dashboard$/); + await page.goto("/data-quality/DQ-DEMO-STATUS"); + + const reviewButton = page.getByRole("button", { name: "Aanbeveling bekijken" }); + await reviewButton.focus(); + await expect(reviewButton).toBeFocused(); + await page.keyboard.press("Enter"); + + const confirmButton = page.getByRole("button", { name: /^Status wijzigen naar/ }); + await expect(confirmButton).toBeVisible(); + + // Status is never conveyed by colour alone: the badge always carries its own text. + const badge = page.locator(".status-decision .badge").first(); + await expect(badge).not.toHaveText(""); + + // The confirm action itself is a real, focusable, keyboard-activatable button (the + // previous "Aanbeveling bekijken" button is unmounted once the decision panel + // replaces it, so focus is verified directly rather than via a Tab chain from it). + await confirmButton.focus(); + await expect(confirmButton).toBeFocused(); + await page.keyboard.press("Enter"); + await expect(page.getByText("Toegepast", { exact: false })).toBeVisible(); +}); diff --git a/frontend/e2e/fleet-ops-correction.spec.ts b/frontend/e2e/fleet-ops-correction.spec.ts new file mode 100644 index 0000000..1bd0ba8 --- /dev/null +++ b/frontend/e2e/fleet-ops-correction.spec.ts @@ -0,0 +1,379 @@ +import { expect, test, type APIRequestContext, type Page } from "@playwright/test"; + +// Targeted end-to-end coverage for the Fleet Ops correction brief (docs/fleet-ops- +// correction/): branding, the redesigned status-recommendation flow (preview/apply/ +// manual-review/stale-token), MO-016 order independence, trilingual knowledge +// grounding, and localized audit/automation content. See also i18n-coverage.spec.ts +// (key parity, brand invariant, translation-quality) and responsive-i18n.spec.ts +// (breakpoint matrix) for the complementary static-content checks. + +async function resetDemoData(request: APIRequestContext) { + const login = await request.post("/api/v1/demo/login", { data: { role: "operations_manager" } }); + expect(login.ok()).toBeTruthy(); + const reset = await request.post("/api/v1/demo/reset"); + expect(reset.ok()).toBeTruthy(); + // /api/v1/demo/reset deletes the session cookie (it recreates the users table), so any + // further authenticated call through this same request context needs a fresh login. + const relogin = await request.post("/api/v1/demo/login", { data: { role: "operations_manager" } }); + expect(relogin.ok()).toBeTruthy(); +} + +const EXPLORE_OPS_MANAGER: Record = { + "nl-BE": "Verken als Operations Manager", + "en-GB": "Explore as Operations Manager", + "fr-BE": "Explorer en tant qu'Operations Manager", +}; + +const REVIEW_RECOMMENDATION: Record = { + "nl-BE": "Aanbeveling bekijken", + "en-GB": "Review recommendation", + "fr-BE": "Voir la recommandation", +}; + +const CHANGE_STATUS_PREFIX: Record = { + "nl-BE": /^Status wijzigen naar/, + "en-GB": /^Change status to/, + "fr-BE": /^Changer le statut vers/, +}; + +const MANUAL_REVIEW_HEADING: Record = { + "nl-BE": "Handmatige beoordeling vereist", + "en-GB": "Manual review required", + "fr-BE": "Évaluation manuelle requise", +}; + +async function loginAsOpsManager(page: Page, lang: string) { + await page.addInitScript((l) => localStorage.setItem("fleetops.language", l), lang); + await page.goto("/login"); + await page.getByRole("button", { name: EXPLORE_OPS_MANAGER[lang] }).click(); + await expect(page).toHaveURL(/\/dashboard$/); +} + +test.describe.configure({ mode: "serial" }); + +test.describe("branding", () => { + for (const lang of ["nl-BE", "en-GB", "fr-BE"]) { + test(`Fleet Ops is the visible brand and no MobilityOps/PoC leaks through (${lang})`, async ({ + page, + request, + }) => { + await resetDemoData(request); + await loginAsOpsManager(page, lang); + await expect(page.locator(".brand-mark").first()).toBeVisible(); + await expect(page.getByText("Fleet Ops", { exact: true }).first()).toBeVisible(); + await expect(page.locator(".app-footer")).toContainText("Fleet Ops"); + await expect(page.locator("html")).toHaveAttribute("lang", lang); + + for (const path of ["/dashboard", "/vehicles", "/data-quality", "/audit", "/automation", "/knowledge"]) { + await page.goto(path); + const text = await page.locator("body").innerText(); + expect(text, `${path} (${lang})`).not.toContain("MobilityOps"); + expect(text, `${path} (${lang})`).not.toMatch(/\bPoC\b/); + } + }); + } +}); + +test("language switcher control changes the UI and persists across a reload", async ({ page, request }) => { + await resetDemoData(request); + // Deliberately not using loginAsOpsManager here: its addInitScript would re-force + // nl-BE on every reload, defeating exactly the persistence behaviour under test. + await page.goto("/login"); + await page.getByRole("button", { name: EXPLORE_OPS_MANAGER["nl-BE"] }).click(); + await expect(page).toHaveURL(/\/dashboard$/); + await expect(page.getByRole("heading", { name: "Aandachtspunten" })).toBeVisible(); + + await page.getByRole("combobox", { name: "Taal" }).selectOption("fr-BE"); + await expect(page.getByRole("heading", { name: "File d'attention" })).toBeVisible(); + await expect(page.locator("html")).toHaveAttribute("lang", "fr-BE"); + + await page.reload(); + await expect(page.getByRole("heading", { name: "File d'attention" })).toBeVisible(); + await expect(page.locator("html")).toHaveAttribute("lang", "fr-BE"); +}); + +test.describe("status-recommendation flow", () => { + test("preview does not mutate anything, apply names the exact target status", async ({ page, request }) => { + await resetDemoData(request); + await loginAsOpsManager(page, "nl-BE"); + await page.goto("/data-quality/DQ-DEMO-STATUS"); + await expect(page.getByRole("heading", { name: "DQ-DEMO-STATUS" })).toBeVisible(); + + await page.getByRole("button", { name: REVIEW_RECOMMENDATION["nl-BE"] }).click(); + await expect(page.getByText("Aanbevolen status")).toBeVisible(); + await expect(page.getByRole("heading", { name: "Waarom" })).toBeVisible(); + await expect(page.getByRole("heading", { name: "Gevolg" })).toBeVisible(); + + // Previewing must not have resolved the issue -- still open, using the page's own + // authenticated session (page.request shares cookies with the browser context). + const issue = await page.request.get("/api/v1/data-quality/issues/DQ-DEMO-STATUS"); + expect((await issue.json()).status).toBe("open"); + + const confirmButton = page.getByRole("button", { name: CHANGE_STATUS_PREFIX["nl-BE"] }); + await expect(confirmButton).toHaveText(/Geblokkeerd/); + await confirmButton.click(); + await expect(page.getByText("Toegepast", { exact: false })).toBeVisible(); + + const resolved = await page.request.get("/api/v1/data-quality/issues/DQ-DEMO-STATUS"); + expect((await resolved.json()).status).toBe("resolved"); + }); + + test("manual review state offers no generic apply button for a genuine fact contradiction", async ({ + page, + request, + }) => { + await resetDemoData(request); + const issues = await ( + await request.get("/api/v1/data-quality/issues", { + params: { rule_type: "vehicle_status_conflict", status: "open" }, + }) + ).json(); + const conflicted = issues.find((i: { entity_ref: string }) => i.entity_ref === "MO-024"); + expect(conflicted, "expected MO-024's vehicle_status_conflict issue to exist after reset").toBeTruthy(); + + await loginAsOpsManager(page, "en-GB"); + await page.goto(`/data-quality/${conflicted.public_ref}`); + await page.getByRole("button", { name: REVIEW_RECOMMENDATION["en-GB"] }).click(); + + await expect(page.getByRole("heading", { name: MANUAL_REVIEW_HEADING["en-GB"] })).toBeVisible(); + await expect(page.getByRole("button", { name: /^Change status to/ })).toHaveCount(0); + }); + + test("a stale recommendation is rejected and the user must review again before applying", async ({ + page, + request, + }) => { + await resetDemoData(request); + await loginAsOpsManager(page, "en-GB"); + await page.goto("/data-quality/DQ-DEMO-STATUS"); + await page.getByRole("button", { name: REVIEW_RECOMMENDATION["en-GB"] }).click(); + await expect(page.getByRole("button", { name: /^Change status to/ })).toBeVisible(); + + // Simulate the underlying facts changing after the preview was shown (the same + // session resolves the booking overlap in the meantime) -- the previously-fetched + // recommendation token must no longer be accepted. + await page.evaluate(async () => { + await fetch("/api/v1/data-quality/issues/DQ-DEMO-OVERLAP/resolve-overlap", { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ booking_ref: "BK-DEMO-OVERLAP-B" }), + }); + }); + + await page.getByRole("button", { name: /^Change status to/ }).click(); + await expect(page.getByText(/situation has changed/i)).toBeVisible(); + await expect(page.getByRole("button", { name: REVIEW_RECOMMENDATION["en-GB"] })).toBeVisible(); + }); +}); + +test.describe("MO-016 status conflict is order-independent", () => { + // Order independence does NOT mean "the same final vehicle status regardless of + // order" -- resolving the booking overlap first genuinely removes the conflict, so + // there is correctly nothing left to apply afterwards. What must hold in either + // order: the recommendation always reflects the real, current facts (never a stale + // "was some other issue open" proxy), and nothing unsafe is ever applied (never + // "rented"). + + test("resolving the booking overlap first correctly leaves nothing to apply", async ({ page, request }) => { + await resetDemoData(request); + await loginAsOpsManager(page, "nl-BE"); + await page.goto("/data-quality/DQ-DEMO-OVERLAP"); + await page.getByRole("radio", { name: /BK-DEMO-OVERLAP-B blokkeren/ }).check(); + await page.getByRole("button", { name: /BK-DEMO-OVERLAP-B blokkeren/ }).click(); + await expect(page.getByText("Opgelost").first()).toBeVisible(); + + await page.goto("/data-quality/DQ-DEMO-STATUS"); + await page.getByRole("button", { name: REVIEW_RECOMMENDATION["nl-BE"] }).click(); + await expect(page.getByRole("heading", { name: "Geen wijziging nodig" })).toBeVisible(); + await expect(page.getByRole("button", { name: CHANGE_STATUS_PREFIX["nl-BE"] })).toHaveCount(0); + + const vehicle = await request.get("/api/v1/vehicles/MO-016"); + expect((await vehicle.json()).operational_status).toBe("available"); + }); + + test("resolving the status conflict first safely blocks the vehicle, unaffected by the later overlap fix", async ({ + page, + request, + }) => { + await resetDemoData(request); + await loginAsOpsManager(page, "nl-BE"); + await page.goto("/data-quality/DQ-DEMO-STATUS"); + await page.getByRole("button", { name: REVIEW_RECOMMENDATION["nl-BE"] }).click(); + await page.getByRole("button", { name: CHANGE_STATUS_PREFIX["nl-BE"] }).click(); + await expect(page.getByText("Toegepast", { exact: false })).toBeVisible(); + + const vehicleMid = await request.get("/api/v1/vehicles/MO-016"); + const statusAfterApply = (await vehicleMid.json()).operational_status; + expect(statusAfterApply).not.toBe("rented"); + + await page.goto("/data-quality/DQ-DEMO-OVERLAP"); + await page.getByRole("radio", { name: /BK-DEMO-OVERLAP-B blokkeren/ }).check(); + await page.getByRole("button", { name: /BK-DEMO-OVERLAP-B blokkeren/ }).click(); + await expect(page.getByText("Opgelost").first()).toBeVisible(); + + // Resolving the now-redundant overlap afterwards must not itself change the + // vehicle's status as a side effect. + const vehicleFinal = await request.get("/api/v1/vehicles/MO-016"); + const statusFinal = (await vehicleFinal.json()).operational_status; + expect(statusFinal).toBe(statusAfterApply); + expect(statusFinal).not.toBe("rented"); + + await resetDemoData(request); + }); +}); + +test.describe("knowledge base is grounded in the operator's own language", () => { + const cases: { lang: string; question: string; sourceHint: RegExp }[] = [ + { + lang: "nl-BE", + question: "Wat moet ik doen wanneer een voertuig beschadigd terugkomt?", + sourceHint: /schadeafhandeling/i, + }, + { + lang: "en-GB", + question: "What should I do when a vehicle returns with damage?", + sourceHint: /damage/i, + }, + { + lang: "fr-BE", + question: "Que dois-je faire lorsqu'un véhicule revient endommagé ?", + sourceHint: /dommages/i, + }, + ]; + + for (const { lang, question, sourceHint } of cases) { + test(`grounded ${lang} answer cites a ${lang} source about damage`, async ({ page, request }) => { + await resetDemoData(request); + await loginAsOpsManager(page, lang); + await page.goto("/knowledge"); + await page.locator("#knowledge-question").fill(question); + await page.getByRole("button", { name: /^(Vraag stellen|Ask|Demander)$/ }).click(); + await expect(page.getByText(sourceHint).first()).toBeVisible({ timeout: 10_000 }); + }); + } +}); + +test("audit trail shows localized action and field labels with raw codes only in technical details", async ({ + page, + request, +}) => { + await resetDemoData(request); + await loginAsOpsManager(page, "nl-BE"); + await page.goto("/data-quality/DQ-DEMO-STATUS"); + await page.getByRole("button", { name: REVIEW_RECOMMENDATION["nl-BE"] }).click(); + await page.getByRole("button", { name: CHANGE_STATUS_PREFIX["nl-BE"] }).click(); + await expect(page.getByText("Toegepast", { exact: false })).toBeVisible(); + + await page.goto("/audit"); + + // Applying resolves both the vehicle status and the issue in one correlated action -- + // expand the group's "technical events" toggle so the other audit row's diff renders + // too, regardless of which one the grouping picked as primary. + const toggle = page.getByRole("button", { name: /technische gebeurtenis/ }).first(); + await expect(toggle).toBeVisible(); + await toggle.click(); + await expect(page.getByRole("button", { name: "Technische gebeurtenissen verbergen" })).toBeVisible(); + + await expect(page.getByText("Aanbevolen status toegepast").first()).toBeVisible(); + const diffs = page.locator(".change-diff"); + await expect(diffs.first()).toBeVisible(); + const combinedDiffText = (await diffs.allInnerTexts()).join(" "); + expect(combinedDiffText).toContain("Operationele status"); + expect(combinedDiffText).not.toContain("operational_status"); +}); + +test("automation shows a localized error explanation with the raw error only under technical details", async ({ + page, + request, +}) => { + await resetDemoData(request); + await loginAsOpsManager(page, "nl-BE"); + await page.goto("/automation"); + + await expect(page.getByText(/tijdelijk niet bereikbaar/).first()).toBeVisible(); + await expect(page.getByText("Synthetic connection timeout to n8n")).not.toBeVisible(); + await page.getByText("Technische details").first().click(); + await expect(page.getByText("Synthetic connection timeout to n8n")).toBeVisible(); +}); + +test.describe("route matrix (section 11F)", () => { + // Opens every main route in all 3 languages: no console errors, correct html[lang], + // and a real, non-empty page heading (proving the route actually rendered content + // instead of silently falling back to a raw i18next key or a blank screen). Key + // parity across locale files is already proven structurally by i18n-coverage.spec.ts + // (every key that exists in nl-BE also exists, non-empty, in en-GB/fr-BE), so this + // matrix focuses on what only a live render can catch. + const routes = [ + "/dashboard", + "/vehicles", + "/vehicles/MO-001", + "/bookings", + "/bookings/BK-DEMO-RETURN", + "/data-quality", + "/data-quality/DQ-DEMO-STATUS", + "/automation", + "/knowledge", + "/audit", + "/scenarios", + "/about", + ]; + + for (const lang of ["nl-BE", "en-GB", "fr-BE"]) { + test(`every main route renders correctly with no console errors (${lang})`, async ({ page, request }) => { + await resetDemoData(request); + + const errors: string[] = []; + page.on("console", (msg) => { + if (msg.type() !== "error") return; + if (msg.text().includes("401") && msg.text().includes("Unauthorized")) return; + errors.push(msg.text()); + }); + page.on("pageerror", (err) => errors.push(err.message)); + + await loginAsOpsManager(page, lang); + + for (const route of routes) { + await page.goto(route); + await expect(page.locator("html")).toHaveAttribute("lang", lang); + const heading = page.getByRole("heading", { level: 1 }); + await expect(heading, `${route} (${lang})`).toBeVisible(); + const headingText = (await heading.first().textContent())?.trim() ?? ""; + expect(headingText, `${route} (${lang}) heading text`).not.toBe(""); + // A raw, unresolved i18next key looks like "namespace:some.key.path" -- real + // page headings never contain a colon followed by a dotted identifier. + expect(headingText, `${route} (${lang}) heading looks like a raw i18n key`).not.toMatch( + /^[a-zA-Z]+:[\w.]+$/, + ); + } + + expect(errors, `Console errors across the route matrix (${lang}):\n${errors.join("\n")}`).toEqual([]); + }); + } +}); + +test.describe("data-quality evidence summary is localized, not raw English (section 6)", () => { + // The primary evidence line at the top of every issue's detail page must render the + // structured `evidence.signals` in the operator's language; the legacy English + // `evidence.summary` string is a technical fallback only, visible solely inside + // "Technical details". Live-caught: this line was unconditionally showing raw + // English ("vehicle marked available while reserved bookings conflict") in every + // language until fixed. + const cases: { lang: string; expectedText: RegExp }[] = [ + { lang: "nl-BE", expectedText: /overlappende reserveringen/i }, + { lang: "en-GB", expectedText: /overlapping bookings/i }, + { lang: "fr-BE", expectedText: /chevauchent|chevauchement/i }, + ]; + + for (const { lang, expectedText } of cases) { + test(`vehicle_status_conflict evidence is localized (${lang})`, async ({ page, request }) => { + await resetDemoData(request); + await loginAsOpsManager(page, lang); + await page.goto("/data-quality/DQ-DEMO-STATUS"); + + const summarySection = page.locator(".record-surface-evidence"); + await expect(summarySection).toBeVisible(); + await expect(summarySection).toContainText(expectedText); + await expect(summarySection).not.toContainText("vehicle marked available while reserved bookings conflict"); + }); + } +}); diff --git a/frontend/e2e/i18n-coverage.spec.ts b/frontend/e2e/i18n-coverage.spec.ts index f5d3079..c83e01c 100644 --- a/frontend/e2e/i18n-coverage.spec.ts +++ b/frontend/e2e/i18n-coverage.spec.ts @@ -71,3 +71,187 @@ test("no locale file contains an empty string value", () => { } } }); + +// --- Brand-invariant: "Fleet Ops" is a fixed constant, never a translation value --- +// (see frontend/src/product.ts and docs/fleet-ops-correction/current-gap-audit.md §1). +// A regression here means someone re-introduced a per-locale brand key/value instead of +// interpolating {{productName}} from the shared constant. + +test("no locale file defines an 'appName' key or the literal brand string", () => { + for (const language of LANGUAGES) { + for (const namespace of namespaces) { + const data = loadNamespace(language, namespace); + const raw = JSON.stringify(data); + expect( + raw.includes("Fleet Ops"), + `${language}/${namespace}.json contains the literal brand string "Fleet Ops" -- ` + + `use {{productName}} interpolation instead so the brand can never drift per locale`, + ).toBe(false); + const keys = collectKeyPaths(data); + expect( + keys.some((k) => k === "appName" || k.endsWith(".appName")), + `${language}/${namespace}.json defines an "appName" key -- the brand name must come ` + + `from the PRODUCT_NAME constant, never a translatable key`, + ).toBe(false); + } + } +}); + +// --- Translation-quality: prove values were actually translated, not copy-pasted --- +// Sleutelpariteit alone doesn't prove translation happened (a locale file could contain +// the literal English string under the right key and still pass). For every "real prose" +// string (>=8 chars, not on the allowlist below), assert nl-BE and fr-BE differ from +// en-GB, and that fr-BE differs from nl-BE -- catching both "still English" and +// "Dutch text copy-pasted into French" in one pass. + +// Exact (namespace, key-path) pairs that are legitimately identical across two or more +// locales: real proper nouns/brand names, deliberately-untranslated role titles, and +// genuine cross-language cognates (identical spelling in Dutch/French/English). This is +// a precise allowlist by key path, not a broad word-level allowlist, so it can't quietly +// hide an unrelated real mistranslation under the same key in a different namespace. +const IDENTICAL_VALUE_ALLOWLIST = new Set([ + "audit.title", // "Audit trail" kept as an established cross-language compliance term + "audit.diff.was", // "{{field}}: was {{value}}" -- "was" is spelled identically in Dutch + "auth.roleOperationsManager", // deliberately-untranslated role title (see demo.json roles) + "auth.roleRentalEmployee", + "demo.scenarios.roles.operations_manager", // same convention, scenario-overview role labels + "demo.scenarios.roles.rental_employee", + "common.language.nl-BE", // language-picker options show each language's own endonym + "common.language.fr-BE", + "common.footer.productLine", // "{{productName}} Demo" -- brief-specified exact footer text + "common.orgName", // "Northstar Mobility" -- fictional org proper noun, same in all 3 + "dashboard.attention.openRecord", // "Open {{title}}" -- "open" is also the Dutch imperative + "demo.scenarios.durationValue", // "± {{minutes}} min" -- unit abbreviation, same in all 3 + "demo.scenarios.startScenario", // "start"/"scenario" are naturalised loanwords in Dutch + "demo.about.limitationsTitle", // "Limitations" -- identical spelling in French + "demo.integrationSummary.titles.mcp_hub", // "ITWorx MCP Hub" -- proper noun + "fleet.list.columns.attention", // "Attention" -- identical spelling in French + "fleet.detail.tabs.inspections", // "Inspections" -- identical spelling in French + "integrations.cards.orchestrationKicker", // "Orchestration" -- identical in French + "knowledge.questionLabel", // "Question" -- identical spelling in French + "knowledge.retrievalFlow.question", + "knowledge.questionLabelExchange", + "navigation.items.audit", // "Audit trail" kept as an established cross-language term + "returns.result.inspection", // "Inspection" -- identical spelling in French +]); + +function isTranslatableProse(value: unknown): value is string { + if (typeof value !== "string") return false; + if (value.trim().length < 8) return false; + // Strip interpolation placeholders and non-letter characters; if nothing substantial + // remains (pure numbers/punctuation/units), it's not "prose" that needs translating. + const stripped = value + .replace(/\{\{[^}]+\}\}/g, " ") + .replace(/[^a-zA-Zà-öø-ÿÀ-ÖØ-ß]/g, ""); + return stripped.trim().length >= 3; +} + +test("nl-BE and fr-BE translations are not suspiciously identical to en-GB or each other", () => { + for (const namespace of namespaces) { + const en = loadNamespace("en-GB", namespace); + const nl = loadNamespace("nl-BE", namespace); + const fr = loadNamespace("fr-BE", namespace); + const keys = collectKeyPaths(en); + + for (const keyPath of keys) { + if (IDENTICAL_VALUE_ALLOWLIST.has(`${namespace}.${keyPath}`)) continue; + const at = (data: Record) => + keyPath.split(".").reduce((acc, part) => { + if (acc && typeof acc === "object") return (acc as Record)[part]; + return undefined; + }, data); + + const enValue = at(en); + if (!isTranslatableProse(enValue)) continue; + const nlValue = at(nl); + const frValue = at(fr); + + expect( + nlValue, + `${namespace}.json:${keyPath} — nl-BE is identical to en-GB ("${enValue}"); ` + + `looks untranslated (add to IDENTICAL_VALUE_ALLOWLIST if this is intentional)`, + ).not.toBe(enValue); + expect( + frValue, + `${namespace}.json:${keyPath} — fr-BE is identical to en-GB ("${enValue}"); ` + + `looks untranslated (add to IDENTICAL_VALUE_ALLOWLIST if this is intentional)`, + ).not.toBe(enValue); + expect( + frValue, + `${namespace}.json:${keyPath} — fr-BE is identical to nl-BE ("${nlValue}"); ` + + `looks like Dutch text was copy-pasted into the French locale`, + ).not.toBe(nlValue); + } + } +}); + +// --- Hardcoded JSX text (section 11D) --- +// A targeted, deliberately narrow static scan: JSX text nodes (`>literal text<`, not a +// `{...}` expression) containing two or more real words are almost always user-facing +// prose that should go through t(...). This is not a full parser, so a short, explicit +// allowlist covers technical tokens/proper nouns that are correctly never translated +// (MobilityOps.md is checked for absence elsewhere; this list is for things that ARE +// expected to appear literally in JSX). +const SRC_DIR = path.resolve(__dirname, "../src"); +const SCAN_DIRS = ["pages", "components"]; + +const ALLOWED_LITERAL_TEXT = new Set([ + "Fleet Ops", // the non-localizable brand name (frontend/src/product.ts) + "Northstar Mobility", // fictional demo org, a proper noun + "ITWorx MCP Hub", // proper noun +]); + +function collectTsxFiles(dir: string): string[] { + const entries = fs.readdirSync(dir, { withFileTypes: true }); + return entries.flatMap((entry) => { + const full = path.join(dir, entry.name); + if (entry.isDirectory()) return collectTsxFiles(full); + return entry.name.endsWith(".tsx") ? [full] : []; + }); +} + +function findHardcodedJsxText(filePath: string): string[] { + const source = fs.readFileSync(filePath, "utf-8"); + const findings: string[] = []; + // Matches `text` where the closing tag name backreferences the + // opening one -- this specifically excludes TypeScript generics like + // `useState(null)`, which have no matching `` closer, + // unlike a naive `>...<` scan would. Deliberately spans newlines (Prettier commonly + // puts JSX text on its own line) and does not attempt to parse JSX properly -- it is + // a fast, approximate net for the common mistake, not a compiler. + const jsxTextPattern = /<([A-Za-z][\w.]*)(?:\s[^<>]*)?>([^<>{}]{3,200})<\/\1>/gs; + let match: RegExpExecArray | null; + while ((match = jsxTextPattern.exec(source)) !== null) { + const text = match[2].trim(); + if (!text) continue; + if (ALLOWED_LITERAL_TEXT.has(text)) continue; + // Needs at least two alphabetic words to count as "prose" -- filters out numbers, + // single technical words, units (km, %), punctuation-only fragments, and JSX + // whitespace artifacts. + const words = text.match(/[A-Za-z]+/g) ?? []; + if (words.length < 2) continue; + // Skip anything that is itself an i18next interpolation artifact leaking through + // (shouldn't happen, but never flag `{{...}}`-shaped remnants) or looks like a URL + // or path. + if (/^https?:\/\//.test(text) || text.includes("/") || text.includes("{{")) continue; + findings.push(`${path.relative(SRC_DIR, filePath)}: "${text}"`); + } + return findings; +} + +test("no hardcoded user-facing JSX text outside the approved technical-token allowlist", () => { + const allFindings: string[] = []; + for (const dir of SCAN_DIRS) { + const files = collectTsxFiles(path.join(SRC_DIR, dir)); + for (const file of files) { + allFindings.push(...findHardcodedJsxText(file)); + } + } + expect( + allFindings, + `Found ${allFindings.length} likely hardcoded JSX string(s) bypassing t(...). ` + + `Either route it through the translation system, or add the exact literal to ` + + `ALLOWED_LITERAL_TEXT in this test if it's a genuine proper noun/technical token:\n` + + allFindings.join("\n"), + ).toEqual([]); +}); diff --git a/frontend/e2e/interactive-elements.spec.ts b/frontend/e2e/interactive-elements.spec.ts index a00160c..c771b80 100644 --- a/frontend/e2e/interactive-elements.spec.ts +++ b/frontend/e2e/interactive-elements.spec.ts @@ -130,9 +130,13 @@ test("return preview correctly reports blocked (not maintenance) for damage repo // The preview is the server's authoritative evaluation: damage always routes to // "blocked", never "maintenance" -- this used to be guessed client-side and wrong. - await expect(page.getByText("Damage was reported on return.")).toBeVisible(); + // The localized reason is the primary text; the raw code sits behind "Technical + // details" so it isn't visible until expanded. + await expect(page.getByText("Damage was reported on this return.")).toBeVisible(); const statusRegion = page.locator(".impact-preview"); await expect(statusRegion.getByText("blocked", { exact: true })).toBeVisible(); + await page.getByText("Technical details").click(); + await expect(page.getByText("Damage was reported on return.")).toBeVisible(); }); test("data quality page: status and rule-type filters work", async ({ page }) => { @@ -201,8 +205,8 @@ test("data quality: applying the recommended status resolves a vehicle conflict" await page.goto("/data-quality/DQ-DEMO-STATUS"); await expect(page.getByRole("heading", { name: "DQ-DEMO-STATUS" })).toBeVisible(); - await page.getByRole("button", { name: "Calculate and apply recommended status" }).click(); - await page.getByRole("button", { name: "Yes, apply" }).click(); + await page.getByRole("button", { name: "Review recommendation" }).click(); + await page.getByRole("button", { name: /^Change status to/ }).click(); await expect(page.getByText("Applied", { exact: false })).toBeVisible(); }); diff --git a/frontend/src/api/types.ts b/frontend/src/api/types.ts index df0bc30..d6de408 100644 --- a/frontend/src/api/types.ts +++ b/frontend/src/api/types.ts @@ -109,6 +109,7 @@ export interface AutomationRun { status: string; attempts: number; last_error: string | null; + last_error_code: string | null; occurred_at: string; } @@ -154,6 +155,8 @@ export interface ReturnPreviewResult { resulting_odometer_km: number; resulting_vehicle_status: string; status_reason: string; + status_reason_code: string; + status_reason_params: Record; would_create_quality_issue: boolean; attention_reasons: string[]; next_booking_risk: NextBookingRisk | null; @@ -173,7 +176,27 @@ export interface DataQualityIssueDetail extends DataQualityIssue { export interface ApplyRecommendedStatusResult { issue: DataQualityIssue; applied_status: string; - reason: string; + reason_code: string; +} + +export interface VehicleStatusFacts { + active_booking_refs: string[]; + overlapping_booking_pairs: string[][]; + service_threshold_reached: boolean; + odometer_km: number; + next_service_km: number; + open_booking_overlap_issue_ref: string | null; +} + +export interface StatusRecommendation { + current_status: string; + recommended_status: string | null; + recommendation_code: string; + safe_to_apply: boolean; + manual_review_required: boolean; + facts: VehicleStatusFacts; + blocking_reasons: string[]; + recommendation_token: string; } export interface ScanResult { @@ -183,7 +206,8 @@ export interface ScanResult { export interface SearchResultItem { type: "vehicle" | "booking" | "data_quality_issue" | "section"; label: string; - detail: string; + detail_code: string; + detail_params: Record; link: string; } diff --git a/frontend/src/components/DemoGuide.tsx b/frontend/src/components/DemoGuide.tsx index 55a7caf..8be19c1 100644 --- a/frontend/src/components/DemoGuide.tsx +++ b/frontend/src/components/DemoGuide.tsx @@ -8,6 +8,7 @@ import { useDemoManifest } from "../context/DemoManifestContext"; import { useViewportTier } from "../hooks/useViewportTier"; import { DEMO_GUIDE_STEPS } from "../data/demoGuideSteps"; import { Icon } from "./Icons"; +import { PRODUCT_NAME } from "../product"; export function DemoGuideTrigger() { const { t } = useTranslation("demo"); @@ -206,7 +207,7 @@ export function DemoGuide() {

{t("guide.whatYouWillSee")}
{t(`guide.steps.${step.id}.whatYouWillSee`)}

{t("guide.whyItMatters")}
{t(`guide.steps.${step.id}.whyItMatters`)}

{t("guide.startAction")}
{t(`guide.steps.${step.id}.startAction`)}

-

{t("guide.expectedOutcome")}
{t(`guide.steps.${step.id}.expectedOutcome`)}

+

{t("guide.expectedOutcome")}
{t(`guide.steps.${step.id}.expectedOutcome`, { productName: PRODUCT_NAME })}

)} diff --git a/frontend/src/components/Layout.tsx b/frontend/src/components/Layout.tsx index 1aa6907..4b05fa5 100644 --- a/frontend/src/components/Layout.tsx +++ b/frontend/src/components/Layout.tsx @@ -10,6 +10,7 @@ import { DemoGuide, DemoGuideTrigger } from "./DemoGuide"; import { LanguageSwitcher } from "./LanguageSwitcher"; import { useDemoGuide } from "../context/DemoGuideContext"; import { useDemoManifest } from "../context/DemoManifestContext"; +import { PRODUCT_NAME } from "../product"; const SEARCH_ICON: Record = { vehicle: "fleet", @@ -18,6 +19,31 @@ const SEARCH_ICON: Record = { section: "chevron", }; +// The backend only ever sends a stable code + raw data params (never English prose) -- +// see app/api/routers/search.py. Localizing here means the label/detail always follow +// the operator's selected locale, in every namespace search results can point to. +function searchResultLabel(t: (key: string, opts?: Record) => string, item: SearchResultItem): string { + if (item.type === "section") return t(`navigation:items.${item.label}`, { defaultValue: item.label }); + return item.label; +} + +function searchResultDetail(t: (key: string, opts?: Record) => string, item: SearchResultItem): string { + switch (item.type) { + case "section": + return t(`searchSections.${item.detail_code}`, { defaultValue: item.detail_code }); + case "vehicle": + return t("searchVehicleSummary", { ...item.detail_params }); + case "booking": + return t(`bookings:statuses.${item.detail_code}`, { defaultValue: item.detail_code }); + case "data_quality_issue": + return t(`quality:ruleTypes.${item.detail_code}`, { + defaultValue: item.detail_code.replace(/_/g, " "), + }); + default: + return item.detail_code; + } +} + interface NavItem { to: string; labelKey: string; @@ -177,7 +203,7 @@ export function Layout() {