Files
NuklearRabbitandClaude Sonnet 5 6deb95524d fix: safe status-recommendation flow, MO-016 order independence, brand constant, message codes
- Add a single shared, pure vehicle-status evaluator (app/services/vehicle_status.py)
  used identically by the data-quality scanner, a new non-mutating status-recommendation
  preview endpoint, and a transactional apply endpoint with optimistic-concurrency token
  revalidation -- eliminates the old opaque "calculate and apply" action and the unsafe
  "maintenance + active booking -> auto rented" shortcut. Frontend
  DataQualityIssueDetail.tsx now shows a review/decide/confirm panel with localized
  why/evidence/consequence text in nl-BE/en-GB/fr-BE, with an exact "Change status to
  <status>" confirm action per the brief.
- Fix MO-016 issue-order dependency: resolving the booking-overlap issue before vs.
  after the status-conflict issue now converges on the same final vehicle status,
  proven by test_mo_016_status_conflict_recommendation_is_order_independent.
- Make "Fleet Ops" a non-localizable brand constant (frontend/src/product.ts,
  backend PRODUCT_NAME) via {{productName}} interpolation everywhere the brand name
  appeared in locale prose; add a permanent test guarding against a translation file
  ever defining the brand name or an "appName" key again.
- Convert dynamic backend prose to stable message codes + params: return status
  reasons, audit field/actor-type labels, automation last_error, and search
  section/vehicle/booking/issue results all now carry codes the frontend localizes,
  with raw technical text demoted to a "Technical details" disclosure.
- docs/fleet-ops-correction/: gap audit, i18n inventory, and the vehicle-status
  decision table documenting the evaluator's rules and safe-status principles.

148 backend tests + Ruff + mypy green; Alembic migration verified upgrade/downgrade;
frontend tsc/build and the i18n-coverage Playwright suite green.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
2026-08-03 21:37:34 +02:00

9.5 KiB
Raw Permalink Blame History

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<dd>{String(issue.evidence.summary ?? "")}</dd> — 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.

35, 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_issueblocked 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.

67. 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:76aria-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.