NuklearRabbitandClaude Sonnet 5
6deb95524d
- 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>
76 lines
9.5 KiB
Markdown
76 lines
9.5 KiB
Markdown
# 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.
|
||
|
||
### 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.
|